From c5bd64ea146162967c29bd2af0cbb845ba3eaaaf Mon Sep 17 00:00:00 2001 From: Young_X Date: Fri, 23 Nov 2018 14:47:36 +0800 Subject: [PATCH 1/8] [MJ2] To avoid divisions by zero / undefined behaviour on shift Signed-off-by: Young_X --- src/lib/openmj2/pi.c | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/src/lib/openmj2/pi.c b/src/lib/openmj2/pi.c index d0fdb9b1..ce9c16e4 100644 --- a/src/lib/openmj2/pi.c +++ b/src/lib/openmj2/pi.c @@ -224,6 +224,13 @@ static opj_bool pi_next_rpcl(opj_pi_iterator_t * pi) try1 = int_ceildiv(pi->ty1, comp->dy << levelno); rpx = res->pdx + levelno; rpy = res->pdy + levelno; + + /* To avoid divisions by zero / undefined behaviour on shift */ + if (rpx >= 31 || ((comp->dx << rpx) >> rpx) != comp->dx || + rpy >= 31 || ((comp->dy << rpy) >> rpy) != comp->dy) { + continue; + } + if (!((pi->y % (comp->dy << rpy) == 0) || ((pi->y == pi->ty0) && ((try0 << levelno) % (1 << rpy))))) { continue; @@ -317,6 +324,13 @@ static opj_bool pi_next_pcrl(opj_pi_iterator_t * pi) try1 = int_ceildiv(pi->ty1, comp->dy << levelno); rpx = res->pdx + levelno; rpy = res->pdy + levelno; + + /* To avoid divisions by zero / undefined behaviour on shift */ + if (rpx >= 31 || ((comp->dx << rpx) >> rpx) != comp->dx || + rpy >= 31 || ((comp->dy << rpy) >> rpy) != comp->dy) { + continue; + } + if (!((pi->y % (comp->dy << rpy) == 0) || ((pi->y == pi->ty0) && ((try0 << levelno) % (1 << rpy))))) { continue; @@ -408,6 +422,13 @@ static opj_bool pi_next_cprl(opj_pi_iterator_t * pi) try1 = int_ceildiv(pi->ty1, comp->dy << levelno); rpx = res->pdx + levelno; rpy = res->pdy + levelno; + + /* To avoid divisions by zero / undefined behaviour on shift */ + if (rpx >= 31 || ((comp->dx << rpx) >> rpx) != comp->dx || + rpy >= 31 || ((comp->dy << rpy) >> rpy) != comp->dy) { + continue; + } + if (!((pi->y % (comp->dy << rpy) == 0) || ((pi->y == pi->ty0) && ((try0 << levelno) % (1 << rpy))))) { continue; From 619e1b086eaa21ebd9b23eb67deee543b07bf06f Mon Sep 17 00:00:00 2001 From: Young_X Date: Fri, 23 Nov 2018 15:02:26 +0800 Subject: [PATCH 2/8] [JPWL] fix CVE-2018-16375 Signed-off-by: Young_X --- src/bin/jpwl/convert.c | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/src/bin/jpwl/convert.c b/src/bin/jpwl/convert.c index 73c1be72..04ca64ca 100644 --- a/src/bin/jpwl/convert.c +++ b/src/bin/jpwl/convert.c @@ -41,6 +41,7 @@ #include #include #include +#include #ifdef OPJ_HAVE_LIBTIFF #include @@ -1862,6 +1863,15 @@ opj_image_t* pnmtoimage(const char *filename, opj_cparameters_t *parameters) return NULL; } + /* This limitation could be removed by making sure to use size_t below */ + if (header_info.height != 0 && + header_info.width > INT_MAX / header_info.height) { + fprintf(stderr, "pnmtoimage:Image %dx%d too big!\n", + header_info.width, header_info.height); + fclose(fp); + return NULL; + } + format = header_info.format; switch (format) { From 46822d0eddc3324b2a056bc60ffa997027bebd66 Mon Sep 17 00:00:00 2001 From: Young_X Date: Fri, 23 Nov 2018 15:58:23 +0800 Subject: [PATCH 3/8] [JPWL] imagetotga(): fix read heap buffer overflow if numcomps < 3 (#987) Signed-off-by: Young_X --- src/bin/jpwl/convert.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/src/bin/jpwl/convert.c b/src/bin/jpwl/convert.c index 04ca64ca..4f636c17 100644 --- a/src/bin/jpwl/convert.c +++ b/src/bin/jpwl/convert.c @@ -445,7 +445,7 @@ int imagetotga(opj_image_t * image, const char *outfile) { int width, height, bpp, x, y; opj_bool write_alpha; - int i, adjustR, adjustG, adjustB; + int i, adjustR, adjustG = 0, adjustB = 0; unsigned int alpha_channel; float r, g, b, a; unsigned char value; @@ -486,8 +486,10 @@ int imagetotga(opj_image_t * image, const char *outfile) scale = 255.0f / (float)((1 << image->comps[0].prec) - 1); adjustR = (image->comps[0].sgnd ? 1 << (image->comps[0].prec - 1) : 0); - adjustG = (image->comps[1].sgnd ? 1 << (image->comps[1].prec - 1) : 0); - adjustB = (image->comps[2].sgnd ? 1 << (image->comps[2].prec - 1) : 0); + if (image->numcomps >= 3) { + adjustG = (image->comps[1].sgnd ? 1 << (image->comps[1].prec - 1) : 0); + adjustB = (image->comps[2].sgnd ? 1 << (image->comps[2].prec - 1) : 0); + } for (y = 0; y < height; y++) { unsigned int index = y * width; From c277159986c80142180fbe5efb256bbf3bdf3edc Mon Sep 17 00:00:00 2001 From: Young_X Date: Fri, 23 Nov 2018 16:12:53 +0800 Subject: [PATCH 4/8] [MJ2] Avoid index out of bounds access to pi->include[] Signed-off-by: Young_X --- src/lib/openmj2/pi.c | 31 +++++++++++++++++++++++++++++++ 1 file changed, 31 insertions(+) diff --git a/src/lib/openmj2/pi.c b/src/lib/openmj2/pi.c index ce9c16e4..f74dc6af 100644 --- a/src/lib/openmj2/pi.c +++ b/src/lib/openmj2/pi.c @@ -85,6 +85,12 @@ static opj_bool pi_next_cprl(opj_pi_iterator_t * pi); ========================================================== */ +static void opj_pi_emit_error(opj_pi_iterator_t * pi, const char* msg) +{ + (void)pi; + (void)msg; +} + static opj_bool pi_next_lrcp(opj_pi_iterator_t * pi) { opj_pi_comp_t *comp = NULL; @@ -114,6 +120,11 @@ static opj_bool pi_next_lrcp(opj_pi_iterator_t * pi) for (pi->precno = pi->poc.precno0; pi->precno < pi->poc.precno1; pi->precno++) { index = pi->layno * pi->step_l + pi->resno * pi->step_r + pi->compno * pi->step_c + pi->precno * pi->step_p; + /* Avoids index out of bounds access with include*/ + if (index >= pi->include_size) { + opj_pi_emit_error(pi, "Invalid access to pi->include"); + return OPJ_FALSE; + } if (!pi->include[index]) { pi->include[index] = 1; return OPJ_TRUE; @@ -156,6 +167,11 @@ static opj_bool pi_next_rlcp(opj_pi_iterator_t * pi) for (pi->precno = pi->poc.precno0; pi->precno < pi->poc.precno1; pi->precno++) { index = pi->layno * pi->step_l + pi->resno * pi->step_r + pi->compno * pi->step_c + pi->precno * pi->step_p; + /* Avoids index out of bounds access with include*/ + if (index >= pi->include_size) { + opj_pi_emit_error(pi, "Invalid access to pi->include"); + return OPJ_FALSE; + } if (!pi->include[index]) { pi->include[index] = 1; return OPJ_TRUE; @@ -256,6 +272,11 @@ static opj_bool pi_next_rpcl(opj_pi_iterator_t * pi) for (pi->layno = pi->poc.layno0; pi->layno < pi->poc.layno1; pi->layno++) { index = pi->layno * pi->step_l + pi->resno * pi->step_r + pi->compno * pi->step_c + pi->precno * pi->step_p; + /* Avoids index out of bounds access with include*/ + if (index >= pi->include_size) { + opj_pi_emit_error(pi, "Invalid access to pi->include"); + return OPJ_FALSE; + } if (!pi->include[index]) { pi->include[index] = 1; return OPJ_TRUE; @@ -356,6 +377,11 @@ static opj_bool pi_next_pcrl(opj_pi_iterator_t * pi) for (pi->layno = pi->poc.layno0; pi->layno < pi->poc.layno1; pi->layno++) { index = pi->layno * pi->step_l + pi->resno * pi->step_r + pi->compno * pi->step_c + pi->precno * pi->step_p; + /* Avoids index out of bounds access with include*/ + if (index >= pi->include_size) { + opj_pi_emit_error(pi, "Invalid access to pi->include"); + return OPJ_FALSE; + } if (!pi->include[index]) { pi->include[index] = 1; return OPJ_TRUE; @@ -454,6 +480,11 @@ static opj_bool pi_next_cprl(opj_pi_iterator_t * pi) for (pi->layno = pi->poc.layno0; pi->layno < pi->poc.layno1; pi->layno++) { index = pi->layno * pi->step_l + pi->resno * pi->step_r + pi->compno * pi->step_c + pi->precno * pi->step_p; + /* Avoids index out of bounds access with include*/ + if (index >= pi->include_size) { + opj_pi_emit_error(pi, "Invalid access to pi->include"); + return OPJ_FALSE; + } if (!pi->include[index]) { pi->include[index] = 1; return OPJ_TRUE; From c58df149900df862806d0e892859b41115875845 Mon Sep 17 00:00:00 2001 From: Young_X Date: Fri, 23 Nov 2018 16:24:19 +0800 Subject: [PATCH 5/8] [OPENJP2] change the way to compute *p_tx0, *p_tx1, *p_ty0, *p_ty1 in function opj_get_encoding_parameters Signed-off-by: Young_X --- src/lib/openjp2/pi.c | 19 +++++++++++-------- 1 file changed, 11 insertions(+), 8 deletions(-) diff --git a/src/lib/openjp2/pi.c b/src/lib/openjp2/pi.c index 91642ee4..4a6ed68e 100644 --- a/src/lib/openjp2/pi.c +++ b/src/lib/openjp2/pi.c @@ -748,6 +748,9 @@ static void opj_get_encoding_parameters(const opj_image_t *p_image, /* position in x and y of tile */ OPJ_UINT32 p, q; + /* non-corrected (in regard to image offset) tile offset */ + OPJ_UINT32 l_tx0, l_ty0; + /* preconditions */ assert(p_cp != 00); assert(p_image != 00); @@ -763,14 +766,14 @@ static void opj_get_encoding_parameters(const opj_image_t *p_image, q = p_tileno / p_cp->tw; /* find extent of tile */ - *p_tx0 = opj_int_max((OPJ_INT32)(p_cp->tx0 + p * p_cp->tdx), - (OPJ_INT32)p_image->x0); - *p_tx1 = opj_int_min((OPJ_INT32)(p_cp->tx0 + (p + 1) * p_cp->tdx), - (OPJ_INT32)p_image->x1); - *p_ty0 = opj_int_max((OPJ_INT32)(p_cp->ty0 + q * p_cp->tdy), - (OPJ_INT32)p_image->y0); - *p_ty1 = opj_int_min((OPJ_INT32)(p_cp->ty0 + (q + 1) * p_cp->tdy), - (OPJ_INT32)p_image->y1); + l_tx0 = p_cp->tx0 + p * + p_cp->tdx; /* can't be greater than p_image->x1 so won't overflow */ + *p_tx0 = (OPJ_INT32)opj_uint_max(l_tx0, p_image->x0); + *p_tx1 = (OPJ_INT32)opj_uint_min(opj_uint_adds(l_tx0, p_cp->tdx), p_image->x1); + l_ty0 = p_cp->ty0 + q * + p_cp->tdy; /* can't be greater than p_image->y1 so won't overflow */ + *p_ty0 = (OPJ_INT32)opj_uint_max(l_ty0, p_image->y0); + *p_ty1 = (OPJ_INT32)opj_uint_min(opj_uint_adds(l_ty0, p_cp->tdy), p_image->y1); /* max precision is 0 (can only grow) */ *p_max_prec = 0; From ce9583d1d7627e007a34a31ae4e22a00d78bd153 Mon Sep 17 00:00:00 2001 From: Young_X Date: Fri, 23 Nov 2018 17:12:06 +0800 Subject: [PATCH 6/8] [JPWL] opj_compress: reorder checks related to code block dimensions to avoid potential int overflow Signed-off-by: Young_X --- src/bin/jpwl/opj_jpwl_compress.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/bin/jpwl/opj_jpwl_compress.c b/src/bin/jpwl/opj_jpwl_compress.c index 28f0670c..c17486a0 100644 --- a/src/bin/jpwl/opj_jpwl_compress.c +++ b/src/bin/jpwl/opj_jpwl_compress.c @@ -952,8 +952,9 @@ static int parse_cmdline_encoder(int argc, char **argv, case 'b': { /* code-block dimension */ int cblockw_init = 0, cblockh_init = 0; sscanf(opj_optarg, "%d,%d", &cblockw_init, &cblockh_init); - if (cblockw_init * cblockh_init > 4096 || cblockw_init > 1024 - || cblockw_init < 4 || cblockh_init > 1024 || cblockh_init < 4) { + if (cblockw_init > 1024 || cblockw_init < 4 || + cblockh_init > 1024 || cblockh_init < 4 || + cblockw_init * cblockh_init > 4096) { fprintf(stderr, "!! Size of code_block error (option -b) !!\n\nRestriction :\n" " * width*height<=4096\n * 4<=width,height<= 1024\n\n"); From bd88611ed9ad7144ec4f3de54790cd848175891b Mon Sep 17 00:00:00 2001 From: Young_X Date: Fri, 23 Nov 2018 17:15:05 +0800 Subject: [PATCH 7/8] [JP3D] To avoid divisions by zero / undefined behaviour on shift (CVE-2018-14423 Signed-off-by: Young_X --- src/lib/openjp3d/pi.c | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/src/lib/openjp3d/pi.c b/src/lib/openjp3d/pi.c index a03be45e..a58ebcc7 100644 --- a/src/lib/openjp3d/pi.c +++ b/src/lib/openjp3d/pi.c @@ -223,6 +223,14 @@ static bool pi_next_rpcl(opj_pi_iterator_t * pi) rpx = res->pdx + levelnox; rpy = res->pdy + levelnoy; rpz = res->pdz + levelnoz; + + /* To avoid divisions by zero / undefined behaviour on shift */ + if (rpx >= 31 || ((comp->dx << rpx) >> rpx) != comp->dx || + rpy >= 31 || ((comp->dy << rpy) >> rpy) != comp->dy || + rpz >= 31 || ((comp->dz << rpz) >> rpz) != comp->dz) { + continue; + } + if ((!(pi->x % (comp->dx << rpx) == 0) || (pi->x == pi->tx0 && (trx0 << levelnox) % (1 << rpx)))) { continue; @@ -329,6 +337,14 @@ static bool pi_next_pcrl(opj_pi_iterator_t * pi) rpx = res->pdx + levelnox; rpy = res->pdy + levelnoy; rpz = res->pdz + levelnoz; + + /* To avoid divisions by zero / undefined behaviour on shift */ + if (rpx >= 31 || ((comp->dx << rpx) >> rpx) != comp->dx || + rpy >= 31 || ((comp->dy << rpy) >> rpy) != comp->dy || + rpz >= 31 || ((comp->dz << rpz) >> rpz) != comp->dz) { + continue; + } + if ((!(pi->x % (comp->dx << rpx) == 0) || (pi->x == pi->tx0 && (trx0 << levelnox) % (1 << rpx)))) { continue; @@ -432,6 +448,14 @@ static bool pi_next_cprl(opj_pi_iterator_t * pi) rpx = res->pdx + levelnox; rpy = res->pdy + levelnoy; rpz = res->pdz + levelnoz; + + /* To avoid divisions by zero / undefined behaviour on shift */ + if (rpx >= 31 || ((comp->dx << rpx) >> rpx) != comp->dx || + rpy >= 31 || ((comp->dy << rpy) >> rpy) != comp->dy || + rpz >= 31 || ((comp->dz << rpz) >> rpz) != comp->dz) { + continue; + } + if ((!(pi->x % (comp->dx << rpx) == 0) || (pi->x == pi->tx0 && (trx0 << levelnox) % (1 << rpx)))) { continue; From 05be3084460e46282ee63f04c72c451f3271fd28 Mon Sep 17 00:00:00 2001 From: Young Xiao Date: Wed, 28 Nov 2018 14:44:06 +0800 Subject: [PATCH 8/8] [JPWL] tgatoimage(): avoid excessive memory allocation attempt, and fixes unaligned load Signed-off-by: Young Xiao --- src/bin/jpwl/convert.c | 41 +++++++++++++++++++++++++++-------------- 1 file changed, 27 insertions(+), 14 deletions(-) diff --git a/src/bin/jpwl/convert.c b/src/bin/jpwl/convert.c index 4f636c17..a4aa8d04 100644 --- a/src/bin/jpwl/convert.c +++ b/src/bin/jpwl/convert.c @@ -99,15 +99,10 @@ struct tga_header { }; #endif /* INFORMATION_ONLY */ -static unsigned short get_ushort(unsigned short val) +/* Returns a ushort from a little-endian serialized value */ +static unsigned short get_tga_ushort(const unsigned char *data) { - -#ifdef OPJ_BIG_ENDIAN - return (((val & 0xff) << 8) + (val >> 8)); -#else - return (val); -#endif - + return data[0] | (data[1] << 8); } #define TGA_HEADER_SIZE 18 @@ -136,15 +131,15 @@ static int tga_readheader(FILE *fp, unsigned int *bits_per_pixel, id_len = (unsigned char)tga[0]; cmap_type = (unsigned char)tga[1]; image_type = (unsigned char)tga[2]; - cmap_index = get_ushort(*(unsigned short*)(&tga[3])); - cmap_len = get_ushort(*(unsigned short*)(&tga[5])); + cmap_index = get_tga_ushort(*(unsigned short*)(&tga[3])); + cmap_len = get_tga_ushort(*(unsigned short*)(&tga[5])); cmap_entry_size = (unsigned char)tga[7]; - x_origin = get_ushort(*(unsigned short*)(&tga[8])); - y_origin = get_ushort(*(unsigned short*)(&tga[10])); - image_w = get_ushort(*(unsigned short*)(&tga[12])); - image_h = get_ushort(*(unsigned short*)(&tga[14])); + x_origin = get_tga_ushort(*(unsigned short*)(&tga[8])); + y_origin = get_tga_ushort(*(unsigned short*)(&tga[10])); + image_w = get_tga_ushort(*(unsigned short*)(&tga[12])); + image_h = get_tga_ushort(*(unsigned short*)(&tga[14])); pixel_depth = (unsigned char)tga[16]; image_desc = (unsigned char)tga[17]; @@ -334,6 +329,24 @@ opj_image_t* tgatoimage(const char *filename, opj_cparameters_t *parameters) color_space = CLRSPC_SRGB; } + /* If the declared file size is > 10 MB, check that the file is big */ + /* enough to avoid excessive memory allocations */ + if (image_height != 0 && image_width > 10000000 / image_height / numcomps) { + char ch; + OPJ_UINT64 expected_file_size = + (OPJ_UINT64)image_width * image_height * numcomps; + long curpos = ftell(f); + if (expected_file_size > (OPJ_UINT64)INT_MAX) { + expected_file_size = (OPJ_UINT64)INT_MAX; + } + fseek(f, (long)expected_file_size - 1, SEEK_SET); + if (fread(&ch, 1, 1, f) != 1) { + fclose(f); + return NULL; + } + fseek(f, curpos, SEEK_SET); + } + subsampling_dx = parameters->subsampling_dx; subsampling_dy = parameters->subsampling_dy;