shithub: jbig2

Info  •  Files  •  Log  •  Branches

jbig2dec: Avoid accessing bytes outside of MMR decoder line.

Previously the file Bug688080.pdf in bug 693798 e.g. has an object 668
containing a JBIG2 bitstream containing an MMR-coded region where the
width of the region is 32 pixels. At one point while decoding this
image, a0 is in the middle of the line and because of the decoded black
and white runs both a1 and a2 end up at the pixel just beyond the end
of the line. At this point jbig2dec would access the byte supposedly
containing this pixel beyond the end of the line, but that is not
allowed. Because this byte was written back unchanged no real harm was
done, but the access was still being performed, triggering software
like valgrind/ASAN that detects buffer overflows.

This commit also reverts the incorrect fix for bug 693798
introduced in commit 46d6b40803cb7a68ceb06b2f71db8cf3f384c2ee
where the allocated image buffer was simply extended by one byte,
thereby accommodating the illegal access.

--- a/jbig2_image.c

+++ b/jbig2_image.c

@@ -59,8 +59,7 @@

         jbig2_free(ctx->allocator, image);

         return NULL;

     }

-    /* Add 1 to accept runs that exceed image width and clamped to width+1 */

-    image->data = jbig2_new(ctx, uint8_t, (int)check + 1);

+    image->data = jbig2_new(ctx, uint8_t, (int)check);

     if (image->data == NULL) {

         jbig2_error(ctx, JBIG2_SEVERITY_FATAL, -1, "could not allocate image data buffer! [stride(%d)*height(%d) bytes]", stride, height);

         jbig2_free(ctx->allocator, image);

--- a/jbig2_mmr.c

+++ b/jbig2_mmr.c

@@ -864,7 +864,8 @@

                     a2 = mmr->width;

                 if (a1 == MINUS1 || a2 < a1)

                     return jbig2_error(ctx, JBIG2_SEVERITY_FATAL, -1, "negative white H run");

-                jbig2_set_bits(dst, a1, a2);

+                if (a1 < mmr->width)

+                    jbig2_set_bits(dst, a1, a2);

                 a0 = a2;

                 /* printf ("H %d %d\n", white_run, black_run); */

             } else {

@@ -878,7 +879,8 @@

                     a2 = mmr->width;

                 if (a0 == MINUS1 || a1 < a0)

                     return jbig2_error(ctx, JBIG2_SEVERITY_FATAL, -1, "negative black H run");

-                jbig2_set_bits(dst, a0, a1);

+                if (a0 < mmr->width)

+                    jbig2_set_bits(dst, a0, a1);

                 a0 = a2;

                 /* printf ("H %d %d\n", black_run, white_run); */

             }

@@ -892,7 +894,8 @@

             if (c) {

                 if (a0 == MINUS1 || b2 < a0)

                     return jbig2_error(ctx, JBIG2_SEVERITY_FATAL, -1, "negative P run");

-                jbig2_set_bits(dst, a0, b2);

+                if (a0 < mmr->width)

+                    jbig2_set_bits(dst, a0, b2);

             }

             a0 = b2;

         }

@@ -904,7 +907,8 @@

             if (c) {

                 if (a0 == MINUS1 || b1 < a0)

                     return jbig2_error(ctx, JBIG2_SEVERITY_FATAL, -1, "negative V(0) run");

-                jbig2_set_bits(dst, a0, b1);

+                if (a0 < mmr->width)

+                    jbig2_set_bits(dst, a0, b1);

             }

             a0 = b1;

             c = !c;

@@ -919,7 +923,8 @@

             if (c) {

                 if (a0 == MINUS1 || b1 + 1 < a0)

                     return jbig2_error(ctx, JBIG2_SEVERITY_FATAL, -1, "negative VR(1) run");

-                jbig2_set_bits(dst, a0, b1 + 1);

+                if (a0 < mmr->width)

+                    jbig2_set_bits(dst, a0, b1 + 1);

             }

             a0 = b1 + 1;

             c = !c;

@@ -934,7 +939,8 @@

             if (c) {

                 if (a0 == MINUS1 || b1 + 2 < a0)

                     return jbig2_error(ctx, JBIG2_SEVERITY_FATAL, -1, "negative VR(2) run");

-                jbig2_set_bits(dst, a0, b1 + 2);

+                if (a0 < mmr->width)

+                    jbig2_set_bits(dst, a0, b1 + 2);

             }

             a0 = b1 + 2;

             c = !c;

@@ -949,7 +955,8 @@

             if (c) {

                 if (a0 == MINUS1 || b1 + 3 < a0)

                     return jbig2_error(ctx, JBIG2_SEVERITY_FATAL, -1, "negative VR(3) run");

-                jbig2_set_bits(dst, a0, b1 + 3);

+                if (a0 < mmr->width)

+                    jbig2_set_bits(dst, a0, b1 + 3);

             }

             a0 = b1 + 3;

             c = !c;

@@ -964,7 +971,8 @@

             if (c) {

                 if (a0 == MINUS1 || b1 - 1 < a0)

                     return jbig2_error(ctx, JBIG2_SEVERITY_FATAL, -1, "negative VL(1) run");

-                jbig2_set_bits(dst, a0, b1 - 1);

+                if (a0 < mmr->width)

+                    jbig2_set_bits(dst, a0, b1 - 1);

             }

             a0 = b1 - 1;

             c = !c;

@@ -979,7 +987,8 @@

             if (c) {

                 if (a0 == MINUS1 || b1 - 2 < a0)

                     return jbig2_error(ctx, JBIG2_SEVERITY_FATAL, -1, "negative VL(2) run");

-                jbig2_set_bits(dst, a0, b1 - 2);

+                if (a0 < mmr->width)

+                    jbig2_set_bits(dst, a0, b1 - 2);

             }

             a0 = b1 - 2;

             c = !c;

@@ -994,7 +1003,8 @@

             if (c) {

                 if (a0 == MINUS1 || b1 - 3 < a0)

                     return jbig2_error(ctx, JBIG2_SEVERITY_FATAL, -1, "negative VL(3) run");

-                jbig2_set_bits(dst, a0, b1 - 3);

+                if (a0 < mmr->width)

+                    jbig2_set_bits(dst, a0, b1 - 3);

             }

             a0 = b1 - 3;

             c = !c;