From d23e0284c9be174cc6daaa594c1139076771e16b Mon Sep 17 00:00:00 2001 From: mayeut Date: Sat, 23 Sep 2023 11:02:54 +0200 Subject: [PATCH] fix: use `opj_uint_ceildiv` instead of `opj_int_ceildiv` when necessary There are a bunch of loc where we can see a usage of `opj_int_ceildiv`: ``` (OPJ_UINT32)opj_int_ceildiv((OPJ_INT32)a, (OPJ_INT32)b); ``` where a & b are `OPJ_UINT32`. This can lead to overflow/underflow for some a/b combinations. Replace those calls by `opj_uint_ceildiv` instead to always get a correct result. This also allows some valid single tile images with huge tile size to be decoded properly. Fix #1438 --- src/lib/openjp2/j2k.c | 62 +++++++++++-------------- tests/nonregression/CMakeLists.txt | 1 + tests/nonregression/md5refs.txt | 1 + tests/nonregression/test_suite.ctest.in | 4 ++ 4 files changed, 33 insertions(+), 35 deletions(-) diff --git a/src/lib/openjp2/j2k.c b/src/lib/openjp2/j2k.c index 6f98d609..9dbba8f1 100644 --- a/src/lib/openjp2/j2k.c +++ b/src/lib/openjp2/j2k.c @@ -2333,10 +2333,8 @@ static OPJ_BOOL opj_j2k_read_siz(opj_j2k_t *p_j2k, } /* Compute the number of tiles */ - l_cp->tw = (OPJ_UINT32)opj_int_ceildiv((OPJ_INT32)(l_image->x1 - l_cp->tx0), - (OPJ_INT32)l_cp->tdx); - l_cp->th = (OPJ_UINT32)opj_int_ceildiv((OPJ_INT32)(l_image->y1 - l_cp->ty0), - (OPJ_INT32)l_cp->tdy); + l_cp->tw = opj_uint_ceildiv(l_image->x1 - l_cp->tx0, l_cp->tdx); + l_cp->th = opj_uint_ceildiv(l_image->y1 - l_cp->ty0, l_cp->tdy); /* Check that the number of tiles is valid */ if (l_cp->tw == 0 || l_cp->th == 0 || l_cp->tw > 65535 / l_cp->th) { @@ -2353,12 +2351,12 @@ static OPJ_BOOL opj_j2k_read_siz(opj_j2k_t *p_j2k, (p_j2k->m_specific_param.m_decoder.m_start_tile_x - l_cp->tx0) / l_cp->tdx; p_j2k->m_specific_param.m_decoder.m_start_tile_y = (p_j2k->m_specific_param.m_decoder.m_start_tile_y - l_cp->ty0) / l_cp->tdy; - p_j2k->m_specific_param.m_decoder.m_end_tile_x = (OPJ_UINT32)opj_int_ceildiv(( - OPJ_INT32)(p_j2k->m_specific_param.m_decoder.m_end_tile_x - l_cp->tx0), - (OPJ_INT32)l_cp->tdx); - p_j2k->m_specific_param.m_decoder.m_end_tile_y = (OPJ_UINT32)opj_int_ceildiv(( - OPJ_INT32)(p_j2k->m_specific_param.m_decoder.m_end_tile_y - l_cp->ty0), - (OPJ_INT32)l_cp->tdy); + p_j2k->m_specific_param.m_decoder.m_end_tile_x = opj_uint_ceildiv( + p_j2k->m_specific_param.m_decoder.m_end_tile_x - l_cp->tx0, + l_cp->tdx); + p_j2k->m_specific_param.m_decoder.m_end_tile_y = opj_uint_ceildiv( + p_j2k->m_specific_param.m_decoder.m_end_tile_y - l_cp->ty0, + l_cp->tdy); } else { p_j2k->m_specific_param.m_decoder.m_start_tile_x = 0; p_j2k->m_specific_param.m_decoder.m_start_tile_y = 0; @@ -7989,10 +7987,8 @@ OPJ_BOOL opj_j2k_setup_encoder(opj_j2k_t *p_j2k, opj_event_msg(p_manager, EVT_ERROR, "Invalid tile height\n"); return OPJ_FALSE; } - cp->tw = (OPJ_UINT32)opj_int_ceildiv((OPJ_INT32)(image->x1 - cp->tx0), - (OPJ_INT32)cp->tdx); - cp->th = (OPJ_UINT32)opj_int_ceildiv((OPJ_INT32)(image->y1 - cp->ty0), - (OPJ_INT32)cp->tdy); + cp->tw = opj_uint_ceildiv(image->x1 - cp->tx0, cp->tdx); + cp->th = opj_uint_ceildiv(image->y1 - cp->ty0, cp->tdy); /* Check that the number of tiles is valid */ if (cp->tw > 65535 / cp->th) { opj_event_msg(p_manager, EVT_ERROR, @@ -10189,10 +10185,8 @@ static OPJ_BOOL opj_j2k_update_image_dimensions(opj_image_t* p_image, return OPJ_FALSE; } - l_img_comp->x0 = (OPJ_UINT32)opj_int_ceildiv((OPJ_INT32)p_image->x0, - (OPJ_INT32)l_img_comp->dx); - l_img_comp->y0 = (OPJ_UINT32)opj_int_ceildiv((OPJ_INT32)p_image->y0, - (OPJ_INT32)l_img_comp->dy); + l_img_comp->x0 = opj_uint_ceildiv(p_image->x0, l_img_comp->dx); + l_img_comp->y0 = opj_uint_ceildiv(p_image->y0, l_img_comp->dy); l_comp_x1 = opj_int_ceildiv((OPJ_INT32)p_image->x1, (OPJ_INT32)l_img_comp->dx); l_comp_y1 = opj_int_ceildiv((OPJ_INT32)p_image->y1, (OPJ_INT32)l_img_comp->dy); @@ -10395,8 +10389,8 @@ OPJ_BOOL opj_j2k_set_decode_area(opj_j2k_t *p_j2k, p_j2k->m_specific_param.m_decoder.m_end_tile_x = l_cp->tw; p_image->x1 = l_image->x1; } else { - p_j2k->m_specific_param.m_decoder.m_end_tile_x = (OPJ_UINT32)opj_int_ceildiv( - p_end_x - (OPJ_INT32)l_cp->tx0, (OPJ_INT32)l_cp->tdx); + p_j2k->m_specific_param.m_decoder.m_end_tile_x = opj_uint_ceildiv(( + OPJ_UINT32)p_end_x - l_cp->tx0, l_cp->tdx); p_image->x1 = (OPJ_UINT32)p_end_x; } @@ -10419,8 +10413,8 @@ OPJ_BOOL opj_j2k_set_decode_area(opj_j2k_t *p_j2k, p_j2k->m_specific_param.m_decoder.m_end_tile_y = l_cp->th; p_image->y1 = l_image->y1; } else { - p_j2k->m_specific_param.m_decoder.m_end_tile_y = (OPJ_UINT32)opj_int_ceildiv( - p_end_y - (OPJ_INT32)l_cp->ty0, (OPJ_INT32)l_cp->tdy); + p_j2k->m_specific_param.m_decoder.m_end_tile_y = opj_uint_ceildiv(( + OPJ_UINT32)p_end_y - l_cp->ty0, l_cp->tdy); p_image->y1 = (OPJ_UINT32)p_end_y; } /* ----- */ @@ -11344,9 +11338,12 @@ static void opj_j2k_dump_MH_info(opj_j2k_t* p_j2k, FILE* out_stream) fprintf(out_stream, "Codestream info from main header: {\n"); - fprintf(out_stream, "\t tx0=%d, ty0=%d\n", p_j2k->m_cp.tx0, p_j2k->m_cp.ty0); - fprintf(out_stream, "\t tdx=%d, tdy=%d\n", p_j2k->m_cp.tdx, p_j2k->m_cp.tdy); - fprintf(out_stream, "\t tw=%d, th=%d\n", p_j2k->m_cp.tw, p_j2k->m_cp.th); + fprintf(out_stream, "\t tx0=%" PRIu32 ", ty0=%" PRIu32 "\n", p_j2k->m_cp.tx0, + p_j2k->m_cp.ty0); + fprintf(out_stream, "\t tdx=%" PRIu32 ", tdy=%" PRIu32 "\n", p_j2k->m_cp.tdx, + p_j2k->m_cp.tdy); + fprintf(out_stream, "\t tw=%" PRIu32 ", th=%" PRIu32 "\n", p_j2k->m_cp.tw, + p_j2k->m_cp.th); opj_j2k_dump_tile_info(p_j2k->m_specific_param.m_decoder.m_default_tcp, (OPJ_INT32)p_j2k->m_private_image->numcomps, out_stream); fprintf(out_stream, "}\n"); @@ -12102,10 +12099,8 @@ OPJ_BOOL opj_j2k_get_tile(opj_j2k_t *p_j2k, l_img_comp->factor = p_j2k->m_private_image->comps[compno].factor; - l_img_comp->x0 = (OPJ_UINT32)opj_int_ceildiv((OPJ_INT32)p_image->x0, - (OPJ_INT32)l_img_comp->dx); - l_img_comp->y0 = (OPJ_UINT32)opj_int_ceildiv((OPJ_INT32)p_image->y0, - (OPJ_INT32)l_img_comp->dy); + l_img_comp->x0 = opj_uint_ceildiv(p_image->x0, l_img_comp->dx); + l_img_comp->y0 = opj_uint_ceildiv(p_image->y0, l_img_comp->dy); l_comp_x1 = opj_int_ceildiv((OPJ_INT32)p_image->x1, (OPJ_INT32)l_img_comp->dx); l_comp_y1 = opj_int_ceildiv((OPJ_INT32)p_image->y1, (OPJ_INT32)l_img_comp->dy); @@ -12485,12 +12480,9 @@ static void opj_get_tile_dimensions(opj_image_t * l_image, *l_width = (OPJ_UINT32)(l_tilec->x1 - l_tilec->x0); *l_height = (OPJ_UINT32)(l_tilec->y1 - l_tilec->y0); - *l_offset_x = (OPJ_UINT32)opj_int_ceildiv((OPJ_INT32)l_image->x0, - (OPJ_INT32)l_img_comp->dx); - *l_offset_y = (OPJ_UINT32)opj_int_ceildiv((OPJ_INT32)l_image->y0, - (OPJ_INT32)l_img_comp->dy); - *l_image_width = (OPJ_UINT32)opj_int_ceildiv((OPJ_INT32)l_image->x1 - - (OPJ_INT32)l_image->x0, (OPJ_INT32)l_img_comp->dx); + *l_offset_x = opj_uint_ceildiv(l_image->x0, l_img_comp->dx); + *l_offset_y = opj_uint_ceildiv(l_image->y0, l_img_comp->dy); + *l_image_width = opj_uint_ceildiv(l_image->x1 - l_image->x0, l_img_comp->dx); *l_stride = *l_image_width - *l_width; *l_tile_offset = ((OPJ_UINT32)l_tilec->x0 - *l_offset_x) + (( OPJ_UINT32)l_tilec->y0 - *l_offset_y) * *l_image_width; diff --git a/tests/nonregression/CMakeLists.txt b/tests/nonregression/CMakeLists.txt index c515ae61..05fce40c 100644 --- a/tests/nonregression/CMakeLists.txt +++ b/tests/nonregression/CMakeLists.txt @@ -84,6 +84,7 @@ set(BLACKLIST_JPEG2000 issue823.jp2 #kdu_jp2info ok issue826.jp2 #inspection reveales that the transformation value is out of range oss-fuzz2785.jp2 #inspection reveales that the transformation value is out of range + issue1438.j2k ) file(GLOB_RECURSE OPJ_DATA_NR_LIST diff --git a/tests/nonregression/md5refs.txt b/tests/nonregression/md5refs.txt index 66fbcda4..b1558024 100644 --- a/tests/nonregression/md5refs.txt +++ b/tests/nonregression/md5refs.txt @@ -397,3 +397,4 @@ c34637a0f218e4074936e0c89534c5b5 tnsot_zero_missing_eoc.png 33aeb45c59383bb4c2d3671f6431c718 Bretagne1_ht_lossy.j2k.png c34637a0f218e4074936e0c89534c5b5 byte.jph.png c34637a0f218e4074936e0c89534c5b5 byte_causal.jhc.png +bc4f704c723329147bf6601a8b113bb2 huge-tile-size.png diff --git a/tests/nonregression/test_suite.ctest.in b/tests/nonregression/test_suite.ctest.in index 60e477cc..6dd70c63 100644 --- a/tests/nonregression/test_suite.ctest.in +++ b/tests/nonregression/test_suite.ctest.in @@ -657,3 +657,7 @@ opj_decompress -i @INPUT_NR_PATH@/htj2k/Bretagne1_ht.j2k -o @TEMP_PATH@/Bretagne opj_decompress -i @INPUT_NR_PATH@/htj2k/Bretagne1_ht_lossy.j2k -o @TEMP_PATH@/Bretagne1_ht_lossy.j2k.png opj_decompress -i @INPUT_NR_PATH@/htj2k/byte.jph -o @TEMP_PATH@/byte.jph.png opj_decompress -i @INPUT_NR_PATH@/htj2k/byte_causal.jhc -o @TEMP_PATH@/byte_causal.jhc.png + +# huge tile size +opj_decompress -i @INPUT_NR_PATH@/huge-tile-size.jp2 -o @TEMP_PATH@/huge-tile-size.png +!opj_decompress -i @INPUT_NR_PATH@/issue1438.j2k -o @TEMP_PATH@/issue1438.png -- 2.30.2