Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Excessive Iteration in opj_t1_encode_cblks (src/lib/openjp2/t1.c) #1059

Closed
ProbeFuzzer opened this issue Feb 4, 2018 · 3 comments · Fixed by #1172
Closed

Excessive Iteration in opj_t1_encode_cblks (src/lib/openjp2/t1.c) #1059

ProbeFuzzer opened this issue Feb 4, 2018 · 3 comments · Fixed by #1172

Comments

@ProbeFuzzer
Copy link

ProbeFuzzer commented Feb 4, 2018

On the latest version (2.3.0) and master branch of openjpeg,
there is an excessive iteration in the opj_t1_encode_cblks function of src/lib/openjp2/t1.c, which could be triggered by the POC below.

Note that processing the POC of only 144 bytes could cost openjpeg more than 15 minutes. We found, in the code, the program is stuck in a 5-level "for" loops of opj_t1_encode_cblks function. The terminating variables of these loops could be manipulated by the input file. Although the variables are quite reasonable (with prc->cw * prc->ch=512, res->pw * res->ph=501) and the actual number of blocks are well under the declared count (i.e., 512), the program gives no response for a long time and it causes denial of service. This issue is different from #996, which has been fixed in commit 5597522.

2105 OPJ_BOOL opj_t1_encode_cblks(opj_t1_t t1,
2111 {
2116 for (compno = 0; compno < tile->numcomps; ++compno) {
2121 for (resno = 0; resno < tilec->numresolutions; ++resno) {
2124 for (bandno = 0; bandno < res->numbands; ++bandno) {
2134 for (precno = 0; precno < res->pw * res->ph; ++precno) {
2137 for (cblkno = 0; cblkno < prc->cw * prc->ch; ++cblkno) {
...
2206 } /
cblkno /
2207 } /
precno /
2208 } /
bandno /
2209 } /
resno /
2210 } /
compno */
2211 return OPJ_TRUE;
2212 }

To reproduce the issue, run: ./opj_compress -n 1 -i $POC -o /tmp/null.j2k
POC: https://github.com/ProbeFuzzer/poc/blob/master/openjpeg/openjpeg_2-3_opj_compress_excessive-iteration_opj_t1_encode_cblks.bmp

Stack trace:
#0 0x00007ffff685a41a in opj_t1_enc_sigpass_step (t1=0x61200000bec0, flagsp=0x61a00001f4ac, datap=0x7fffdc57a634, bpno=0, one=64, nmsedec=0x7fffffff7c90, type=0 '\000',
ci=3, vsc=0) at /u/youwei/ProbeFuzzer/product/openjpeg/tst/src/src/lib/openjp2/t1.c:346
#1 0x00007ffff68605ab in opj_t1_enc_sigpass (t1=0x61200000bec0, bpno=0, nmsedec=0x7fffffff7c90, type=0 '\000', cblksty=0)
at /u/youwei/ProbeFuzzer/product/openjpeg/tst/src/src/lib/openjp2/t1.c:512
#2 0x00007ffff68ff1f8 in opj_t1_encode_cblk (t1=0x61200000bec0, cblk=0x62d0002d7f80, orient=0, compno=0, level=0, qmfbid=1, stepsize=1, cblksty=0, numcomps=3,
tile=0x61800000fc80, mct_norms=0x7ffff695dc60 <opj_mct_norms>, mct_numcomps=3) at /u/youwei/ProbeFuzzer/product/openjpeg/tst/src/src/lib/openjp2/t1.c:2319
#3 0x00007ffff68fe3f1 in opj_t1_encode_cblks (t1=0x61200000bec0, tile=0x61800000fc80, tcp=0x62200001c900, mct_norms=0x7ffff695dc60 <opj_mct_norms>, mct_numcomps=3)
at /u/youwei/ProbeFuzzer/product/openjpeg/tst/src/src/lib/openjp2/t1.c:2192
#4 0x00007ffff692a74a in opj_tcd_t1_encode (p_tcd=0x60b00000af90) at /u/youwei/ProbeFuzzer/product/openjpeg/tst/src/src/lib/openjp2/tcd.c:2505
#5 0x00007ffff691dcf1 in opj_tcd_encode_tile (p_tcd=0x60b00000af90, p_tile_no=0, p_dest=0x7fff4947280e '\276' <repeats 200 times>..., p_data_written=0x7fffffff7fa0,
p_max_length=688128595, p_cstr_info=0x0, p_manager=0x610000007ea0) at /u/youwei/ProbeFuzzer/product/openjpeg/tst/src/src/lib/openjp2/tcd.c:1408
#6 0x00007ffff67ccadf in opj_j2k_write_sod (p_j2k=0x61300000de80, p_tile_coder=0x60b00000af90, p_data=0x7fff4947280e '\276' <repeats 200 times>...,
p_data_written=0x7fffffff7fa0, p_total_data_size=688128599, p_stream=0x60c00000bf80, p_manager=0x610000007ea0)
at /u/youwei/ProbeFuzzer/product/openjpeg/tst/src/src/lib/openjp2/j2k.c:4685
#7 0x00007ffff68147bc in opj_j2k_write_first_tile_part (p_j2k=0x61300000de80, p_data=0x7fff4947280c "\377\223", '\276' <repeats 198 times>...,
p_data_written=0x7fffffff8060, p_total_data_size=688128599, p_stream=0x60c00000bf80, p_manager=0x610000007ea0)
at /u/youwei/ProbeFuzzer/product/openjpeg/tst/src/src/lib/openjp2/j2k.c:11764
#8 0x00007ffff68131be in opj_j2k_post_write_tile (p_j2k=0x61300000de80, p_stream=0x60c00000bf80, p_manager=0x610000007ea0)
at /u/youwei/ProbeFuzzer/product/openjpeg/tst/src/src/lib/openjp2/j2k.c:11521
#9 0x00007ffff68109e0 in opj_j2k_encode (p_j2k=0x61300000de80, p_stream=0x60c00000bf80, p_manager=0x610000007ea0)
at /u/youwei/ProbeFuzzer/product/openjpeg/tst/src/src/lib/openjp2/j2k.c:11270
#10 0x00007ffff683604e in opj_encode (p_info=0x610000007e40, p_stream=0x60c00000bf80) at /u/youwei/ProbeFuzzer/product/openjpeg/tst/src/src/lib/openjp2/openjpeg.c:817
#11 0x000000000040f4b6 in main (argc=7, argv=0x7fffffffdcf8) at /u/youwei/ProbeFuzzer/product/openjpeg/tst/src/src/bin/jp2/opj_compress.c:1995

@carnil
Copy link

carnil commented Feb 5, 2018

This issue was assigned CVE-2018-6616

@amontalban
Copy link

Hey guys, do you need help with this? Will be any new release anytime soon (Would be great to have this issue addressed). Thanks!

@hlef
Copy link
Contributor

hlef commented Nov 26, 2018

$ file poc.bmp
poc.bmp: PC bitmap, Windows 98/2000 and newer format, 16384001 x 10 x 8

This is most likely a small stack overflow due to the bmp file maliciously advertising a wrong, very large width.

This can usually be fixed by comparing actual and advertised size after reading the file, e.g.

--- a/src/bin/jp2/convertbmp.c
+++ b/src/bin/jp2/convertbmp.c
@@ -534,14 +534,14 @@ static OPJ_BOOL bmp_read_raw_data(FILE* IN, OPJ_UINT8* pData, OPJ_UINT32 stride,
 static OPJ_BOOL bmp_read_rle8_data(FILE* IN, OPJ_UINT8* pData,
                                    OPJ_UINT32 stride, OPJ_UINT32 width, OPJ_UINT32 height)
 {
-    OPJ_UINT32 x, y;
+    OPJ_UINT32 x, y, written;
     OPJ_UINT8 *pix;
     const OPJ_UINT8 *beyond;

     beyond = pData + stride * height;
     pix = pData;

-    x = y = 0U;
+    x = y = written = 0U;
     while (y < height) {
         int c = getc(IN);
         if (c == EOF) {
@@ -561,6 +561,7 @@ static OPJ_BOOL bmp_read_rle8_data(FILE* IN, OPJ_UINT8* pData,
             for (j = 0; (j < c) && (x < width) &&
                     ((OPJ_SIZE_T)pix < (OPJ_SIZE_T)beyond); j++, x++, pix++) {
                 *pix = c1;
+                written++;
             }
         } else {
             c = getc(IN);
@@ -598,6 +599,7 @@ static OPJ_BOOL bmp_read_rle8_data(FILE* IN, OPJ_UINT8* pData,
                     }
                     c1 = (OPJ_UINT8)c1_int;
                     *pix = c1;
+                    written++;
                 }
                 if ((OPJ_UINT32)c & 1U) { /* skip padding byte */
                     c = getc(IN);
@@ -608,6 +610,10 @@ static OPJ_BOOL bmp_read_rle8_data(FILE* IN, OPJ_UINT8* pData,
             }
         }
     }/* while() */
+
+    if (written != width * height)
+        fprintf(stderr, "warning, image's actual size does not match advertised one\n");
+
     return OPJ_TRUE;
 }

I'll take a closer look later and PR a patch once ready.

hlef added a commit to hlef/openjpeg that referenced this issue Dec 14, 2018
width/length dimensions read from bmp headers are not necessarily
valid. For instance they may have been maliciously set to very large
values with the intention to cause DoS (large memory allocation, stack
overflow). In these cases we want to detect the invalid size as early
as possible.

This commit introduces a counter which verifies that the number of
written bytes corresponds to the advertized width/length.

Fixes uclouvain#1059 (CVE-2018-6616).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants