Heap buffer overflow in opj_t1_clbl_decode_processor() triggered with Ghostscript

[fumfel]

Version: 2.3.0

Command to reproduce (Ghostscript): gs -dNOPAUSE -sDEVICE=bit -sOUTPUTFILE=/dev/null -dSAFER gs_hbo_opj_t1_clbl_decode_processor -c quit

Crashing test case (please unpack): gs_hbo_opj_t1_clbl_decode_processor.zip

ASAN log:

==5834==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x7f56033c0440 at pc 0x55beea74fe6c bp 0x7ffd95c8bfa0 sp 0x7ffd95c8bf90
WRITE of size 4 at 0x7f56033c0440 thread T0
    #0 0x55beea74fe6b in opj_t1_clbl_decode_processor openjpeg/src/lib/openjp2/t1.c:1782
    #1 0x55beea7a6205 in opj_thread_pool_submit_job openjpeg/src/lib/openjp2/thread.c:833
    #2 0x55beea753023 in opj_t1_decode_cblks openjpeg/src/lib/openjp2/t1.c:1901
    #3 0x55beea7912e6 in opj_tcd_t1_decode openjpeg/src/lib/openjp2/tcd.c:1963
    #4 0x55beea7912e6 in opj_tcd_decode_tile openjpeg/src/lib/openjp2/tcd.c:1617
    #5 0x55beea68c150 in opj_j2k_decode_tile openjpeg/src/lib/openjp2/j2k.c:8935
    #6 0x55beea68c150 in opj_j2k_decode_tiles openjpeg/src/lib/openjp2/j2k.c:10722
    #7 0x55beea643c8f in opj_j2k_exec openjpeg/src/lib/openjp2/j2k.c:8096
    #8 0x55beea699500 in opj_j2k_decode openjpeg/src/lib/openjp2/j2k.c:11017
    #9 0x55beea6b4a44 in opj_jp2_decode openjpeg/src/lib/openjp2/jp2.c:1604
    #10 0x55beea5e9a45 in decode_image base/sjpx_openjpeg.c:407
    #11 0x55beea5e9a45 in s_opjd_process base/sjpx_openjpeg.c:734
    #12 0x55beea9d3b79 in sreadbuf base/stream.c:823
    #13 0x55beea9e4a98 in s_process_read_buf base/stream.c:749
    #14 0x55beec45367a in image_file_continue psi/zimage.c:533
    #15 0x55beec2751b8 in interp psi/interp.c:1256
    #16 0x55beec2751b8 in gs_call_interp psi/interp.c:516
    #17 0x55beec284c0d in gs_interpret psi/interp.c:473
    #18 0x55beec221e2f in gs_main_interpret psi/imain.c:235
    #19 0x55beec221e2f in gs_main_run_string_end psi/imain.c:658
    #20 0x55beec221e2f in gs_main_run_string_with_length psi/imain.c:610
    #21 0x55beec221e2f in gs_main_run_string psi/imain.c:591
    #22 0x55beec22f0e8 in run_string psi/imainarg.c:1034
    #23 0x55beec22f0e8 in runarg psi/imainarg.c:1024
    #24 0x55beec23898b in argproc psi/imainarg.c:957
    #25 0x55beec23898b in gs_main_init_with_args psi/imainarg.c:233
    #26 0x55bee9a9e249 in main psi/gs.c:95
    #27 0x7f561eaebb96 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x21b96)
    #28 0x55bee9ab3289 in _start (XYZ/gs_asan/bin/gs+0x36a289)

0x7f56033c0441 is located 0 bytes to the right of 261185-byte region [0x7f5603380800,0x7f56033c0441)
allocated by thread T0 here:
    #0 0x7f56203d5b50 in __interceptor_malloc (/usr/lib/x86_64-linux-gnu/libasan.so.4+0xdeb50)
    #1 0x55beeb7bb9c9 in gs_heap_alloc_bytes base/gsmalloc.c:193

SUMMARY: AddressSanitizer: heap-buffer-overflow openjpeg/src/lib/openjp2/t1.c:1782 in opj_t1_clbl_decode_processor
Shadow bytes around the buggy address:
  0x0feb40670030: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0feb40670040: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0feb40670050: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0feb40670060: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0feb40670070: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
=>0x0feb40670080: 00 00 00 00 00 00 00 00[01]fa fa fa fa fa fa fa
  0x0feb40670090: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0feb406700a0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0feb406700b0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0feb406700c0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0feb406700d0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
Shadow byte legend (one shadow byte represents 8 application bytes):
  Addressable:           00
  Partially addressable: 01 02 03 04 05 06 07 
  Heap left redzone:       fa
  Freed heap region:       fd
  Stack left redzone:      f1
  Stack mid redzone:       f2
  Stack right redzone:     f3
  Stack after return:      f5
  Stack use after scope:   f8
  Global redzone:          f9
  Global init order:       f6
  Poisoned by user:        f7
  Container overflow:      fc
  Array cookie:            ac
  Intra object redzone:    bb
  ASan internal:           fe
  Left alloca redzone:     ca
  Right alloca redzone:    cb
==5834==ABORTING

[szukw000]

@fumfel ,
I used gs version 9.25 .
winfried
gs.txt.zip

[rouault]

I've extracted the JP2 content from the .gs file (
gs_hbo_opj_t1_clbl_decode_processor_jp2.jp2.zip )
, and used it with opj_decompress with a -fsanitize=address build of openjpeg v2.3.0 and master, and I don't get any corruption

$ bin/opj_decompress -i  gs_hbo_opj_t1_clbl_decode_processor_jp2.jp2 -o out.tif

[INFO] JP2 IHDR box: compression type indicate that the file is not a conforming JP2 file (48) 
[INFO] Start to read j2k main header (85).
[WARNING] Despite JP2 BPC!=255, precision and/or sgnd values for comp[1] is different than comp[0]:
        [0] prec(8) sgnd(0) [1] prec(8) sgnd(1)
[INFO] Main header has been correctly decoded.
[INFO] No decoded area parameters, set the decoded area to the whole image
[INFO] Psot value of the current tile-part is equal to zero, we assuming it is the last tile-part of the codestream.
[ERROR] Tile part length size inconsistent with stream length
[ERROR] Failed to decode the codestream in the JP2 file
ERROR -> opj_decompress: failed to decode image!

[rouault]

So line t1.c:1782 shouldn't even been executed (I've verified it is not with the above opj_decompress invokation). There must be something specific in the ghostscript invokation/integration

[sebras]

When you extracted the JP2 content, did you extract from offset 73 until EOF? The PDF has been fuzzed so it is lacking the /Length field for the PDF object stream. This means that the stream is considered to continue until EOF (even though the data looks like PDF objects).

While debugging this I noticed that the JP2H box contains two subboxes of types IHDR and 0000. IHDR is parsed correctly but the box of type 0000 causes JP2_STATE_UNKNOWN to be set in jp2_state. As far as I can tell from the spec it indicates that

Other boxes may be defined in other standards and may be ignored by conforming readers.

So perhaps this is correct behaviour?

Next after the JP2H box follows a JP2C box with the length field set to 808464432 bytes. This is larger than the remainder of the file but opj_jp2_read_header_procedure() never checks this. As soon as it sees that the box type is JP2C it is satisfied and exits. I believe that this length field should be checked compared to the size of the remaining data. Is that possible?

The spec does allow for box lengths being 0, implying unknown size. Since the JP2 code stream commonly ends with 0xffd9 I guess this might be used to determine the box size after scanning the code stream, but opj_jp2_read_header_procedure() doesn't seem to handle this. Should it?

I do know why Ghostscript crashes and opj_decompress does not -- opj_decompress implements a hack that Ghostscript lacks. Instead Ghostscript assumes that if openjpeg reports three components, then all three components have data present. This is not the case hence Ghostscript dreferences NULL and crashes.

In the hack opj_decompress checks if there is any data for the first component. Why is this only the first component checked? Shouldn't all components be checked? For this particular JP2 the first two components both lack data, but the third component data is actually present!

In either case I agree with the FIXME comment next to that hack, this simply feels like the wrong solution since every user of openjpeg must now implement the same hack. If the JP2 file states that it has three components and they are not all populated with data after attempting to decode the image then something in openjpeg ought to return an error code to the benefit of all users of openjpeg. I haven't yet determined where this ought to be done, can you help me in finding out what location would be suitable?

[sebras]

I just found that in opj_j2k_read_sod() openjpeg attempts to assume that the last SOT marker extends until EOF (well, at EOF shy of 2 bytes, hoping for EOC or 0xffd9). That seems reasonable to me.

[sebras]

Let me supply some example-files.zip. The first file 7000088.jp2 contains gs_hbo_opj_t1_clbl_decode_processor from the original report but with the trailing parts still present.

Running openjpeg

opj_decompress -i 700088.jp2 -o test.pgm

including the previous supplied pull request causes it to print:

Error reading SPCod SPCoc element, Invalid transformation found

for this file. This is correct since the transformation value is neither 0x00 nor 0x01 as allowed by the specification, but rather 0x30. However I can manually fuzz the file to change the tranformation parameter to be 0x00, this is exactly what is in 7000088-2.jpg from example-files.zip above. With this file running openjpeg:

opj_decompress -i 700088.jp2 -o test.pgm

still causes out of bounds accesses if the previously mentioned hack is not included. I'm investigating further.

[sebras]

Ok, so I have done a commit that I'd like for you guys to take a look at. It errors out whenever not all components requested by the caller have been decoded. This means that the hack in opj_decompress is no longer needed (as it will simply be handled by the library and normal error handling in opj_decompress).

I believe this to be an improvement, but perhaps you want to resolve it differently. I'm open to any suggestions and will rework the set of commits if needed. :)