Avoid heap-buffer-overflow read in non-strict mode

Fixes #1535
uclouvain · Jun 24, 2024 · f2a9410 · f2a9410
1 parent dea92ee
commit f2a9410
Show file tree

Hide file tree

Showing 3 changed files with 27 additions and 20 deletions.
diff --git a/src/lib/openjp2/t1.c b/src/lib/openjp2/t1.c
@@ -2006,10 +2006,16 @@ static OPJ_BOOL opj_t1_decode_cblk(opj_t1_t *t1,
     opj_mqc_setstate(mqc, T1_CTXNO_AGG, 0, 3);
     opj_mqc_setstate(mqc, T1_CTXNO_ZC, 0, 4);
 
+    if (cblk->corrupted) {
+        assert(cblk->numchunks == 0);
+        return OPJ_TRUE;
+    }
+
     /* Even if we have a single chunk, in multi-threaded decoding */
     /* the insertion of our synthetic marker might potentially override */
     /* valid codestream of other codeblocks decoded in parallel. */
-    if (cblk->numchunks > 1 || t1->mustuse_cblkdatabuffer) {
+    if (cblk->numchunks > 1 || (t1->mustuse_cblkdatabuffer &&
+                                cblk->numchunks > 0)) {
         OPJ_UINT32 i;
         OPJ_UINT32 cblk_len;
 

diff --git a/src/lib/openjp2/t2.c b/src/lib/openjp2/t2.c
@@ -1407,18 +1407,21 @@ static OPJ_BOOL opj_t2_read_packet_data(opj_t2_t* p_t2,
         l_nb_code_blocks = l_prc->cw * l_prc->ch;
         l_cblk = l_prc->cblks.dec;
 
-        for (cblkno = 0; cblkno < l_nb_code_blocks; ++cblkno) {
+        for (cblkno = 0; cblkno < l_nb_code_blocks; ++cblkno, ++l_cblk) {
             opj_tcd_seg_t *l_seg = 00;
 
-            // if we have a partial data stream, set numchunks to zero
-            // since we have no data to actually decode.
-            if (partial_buffer) {
-                l_cblk->numchunks = 0;
-            }
-
             if (!l_cblk->numnewpasses) {
                 /* nothing to do */
-                ++l_cblk;
+                continue;
+            }
+
+            if (partial_buffer || l_cblk->corrupted) {
+                /* if a previous segment in this packet couldn't be decoded,
+                 * or if this code block was corrupted in a previous layer,
+                 * then mark it as corrupted.
+                 */
+                l_cblk->numchunks = 0;
+                l_cblk->corrupted = OPJ_TRUE;
                 continue;
             }
 
@@ -1451,18 +1454,13 @@ static OPJ_BOOL opj_t2_read_packet_data(opj_t2_t* p_t2,
                                       "read: segment too long (%d) with max (%d) for codeblock %d (p=%d, b=%d, r=%d, c=%d)\n",
                                       l_seg->newlen, p_max_length, cblkno, p_pi->precno, bandno, p_pi->resno,
                                       p_pi->compno);
-                        // skip this codeblock since it is a partial read
+                        /* skip this codeblock (and following ones in this
+                         * packet) since it is a partial read
+                         */
                         partial_buffer = OPJ_TRUE;
+                        l_cblk->corrupted = OPJ_TRUE;
                         l_cblk->numchunks = 0;
-
-                        l_seg->numpasses += l_seg->numnewpasses;
-                        l_cblk->numnewpasses -= l_seg->numnewpasses;
-                        if (l_cblk->numnewpasses > 0) {
-                            ++l_seg;
-                            ++l_cblk->numsegs;
-                            break;
-                        }
-                        continue;
+                        break;
                     }
                 }
 
@@ -1519,7 +1517,7 @@ static OPJ_BOOL opj_t2_read_packet_data(opj_t2_t* p_t2,
             } while (l_cblk->numnewpasses > 0);
 
             l_cblk->real_num_segs = l_cblk->numsegs;
-            ++l_cblk;
+
         } /* next code_block */
 
         ++l_band;
@@ -1603,6 +1601,8 @@ static OPJ_BOOL opj_t2_skip_packet_data(opj_t2_t* p_t2,
                                       "skip: segment too long (%d) with max (%d) for codeblock %d (p=%d, b=%d, r=%d, c=%d)\n",
                                       l_seg->newlen, p_max_length, cblkno, p_pi->precno, bandno, p_pi->resno,
                                       p_pi->compno);
+
+                        *p_data_read = p_max_length;
                         return OPJ_TRUE;
                     }
                 }

diff --git a/src/lib/openjp2/tcd.h b/src/lib/openjp2/tcd.h
@@ -141,6 +141,7 @@ typedef struct opj_tcd_cblk_dec {
     OPJ_UINT32 numchunksalloc;      /* Number of chunks item allocated */
     /* Decoded code-block. Only used for subtile decoding. Otherwise tilec->data is directly updated */
     OPJ_INT32* decoded_data;
+    OPJ_BOOL corrupted; /* whether the code block data is corrupted */
 } opj_tcd_cblk_dec_t;
 
 /** Precinct structure */