fix: use `opj_uint_ceildiv` instead of `opj_int_ceildiv` when necessary
authormayeut <mayeut@users.noreply.github.com>
Sat, 23 Sep 2023 09:02:54 +0000 (11:02 +0200)
committermayeut <mayeut@users.noreply.github.com>
Sat, 23 Sep 2023 10:50:57 +0000 (12:50 +0200)
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
tests/nonregression/CMakeLists.txt
tests/nonregression/md5refs.txt
tests/nonregression/test_suite.ctest.in

index 6f98d6099cf9b51dcdda4be425f357956d819368..9dbba8f1be3f3108b779ec7719ceec1e1d4246ab 100644 (file)
@@ -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;
index c515ae61835e3203b424cabe2091bced622134e8..05fce40ce271430ac476561b48be81afe7af8e3c 100644 (file)
@@ -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
index 66fbcda45a25b4a99defad5c29182555519c0ef5..b15580247cea2bab7a0bb841c02572a8657e99d8 100644 (file)
@@ -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
index 60e477cc81aea31f432828ff1a1a30669f4ec976..6dd70c632e5fa1732d59d69709a9c95ca6a3879b 100644 (file)
@@ -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