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

Heap-buffer-overflow in opj_dwt_decode #399

Closed
gcode-importer opened this issue Sep 17, 2014 · 12 comments
Closed

Heap-buffer-overflow in opj_dwt_decode #399

gcode-importer opened this issue Sep 17, 2014 · 12 comments

Comments

@gcode-importer
Copy link

Originally reported on Google Code with ID 399

issue 414525: Heap-buffer-overflow in opj_dwt_decode
    http://code.google.com/p/chromium/issues/detail?id=414525

Reported by detonin on 2014-09-17 09:11:54

@gcode-importer
Copy link
Author

Reported by detonin on 2014-09-17 09:17:09

  • Labels added: OpjVersion-2.x

@gcode-importer
Copy link
Author

Reproduced on trunk r2885

./bin/opj_decompress -i ../../data/issue399/0.jp2 -o 0.bmp

===========================================
The extension of this file is incorrect.
FOUND .jp2. SHOULD BE .j2k or .jpc or .j2c
===========================================

[INFO] Start to read j2k main header (0).
[INFO] Main header has been correctly decoded.
[INFO] No decoded area parameters, set the decoded area to the whole image
[INFO] Header of tile 0 / 8 has been read.
[INFO] Tile 1/9 has been decoded.
[INFO] Image data has been updated with tile 1.

[INFO] Header of tile 1 / 8 has been read.
[INFO] Tile 2/9 has been decoded.
[INFO] Image data has been updated with tile 2.

[INFO] Header of tile 2 / 8 has been read.
[INFO] Tile 3/9 has been decoded.
[INFO] Image data has been updated with tile 3.

[INFO] Header of tile 3 / 8 has been read.
[INFO] Tile 4/9 has been decoded.
[INFO] Image data has been updated with tile 4.

[INFO] Header of tile 4 / 8 has been read.
=================================================================
==33814==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x036035d8 at pc
0x0080f723 bp 0xbff1d248 sp 0xbff1d244
WRITE of size 4 at 0x036035d8 thread T0
    #0 0x80f722 in opj_dwt_interleave_h /Users/Matt/Dev/OpenJpeg/issue391/src/lib/openjp2/dwt.c:248:7
    #1 0x80bef8 in opj_dwt_decode_tile /Users/Matt/Dev/OpenJpeg/issue391/src/lib/openjp2/dwt.c:595:4
    #2 0x80b9e0 in opj_dwt_decode /Users/Matt/Dev/OpenJpeg/issue391/src/lib/openjp2/dwt.c:475:9
    #3 0x87b5e7 in opj_tcd_dwt_decode /Users/Matt/Dev/OpenJpeg/issue391/src/lib/openjp2/tcd.c:1550:31
    #4 0x87b04b in opj_tcd_decode_tile /Users/Matt/Dev/OpenJpeg/issue391/src/lib/openjp2/tcd.c:1242:20
    #5 0x827e17 in opj_j2k_decode_tile /Users/Matt/Dev/OpenJpeg/issue391/src/lib/openjp2/j2k.c:7796:15
    #6 0x83c597 in opj_j2k_decode_tiles /Users/Matt/Dev/OpenJpeg/issue391/src/lib/openjp2/j2k.c:9305:23
    #7 0x823f27 in opj_j2k_exec /Users/Matt/Dev/OpenJpeg/issue391/src/lib/openjp2/j2k.c:7187:41
    #8 0x82d8b3 in opj_j2k_decode /Users/Matt/Dev/OpenJpeg/issue391/src/lib/openjp2/j2k.c:9496:15
    #9 0x84ef63 in opj_decode /Users/Matt/Dev/OpenJpeg/issue391/src/lib/openjp2/openjpeg.c:412:10
    #10 0xe586c in main /Users/Matt/Dev/OpenJpeg/issue391/src/bin/jp2/opj_decompress.c:821:10
    #11 0x94511700 in start (/usr/lib/system/libdyld.dylib+0x3700)
    #12 0x4 (<unknown module>)

0x036035d8 is located 4 bytes to the right of 404-byte region [0x03603440,0x036035d4)
allocated by thread T0 here:
    #0 0x348dba in wrap_malloc (/Users/Matt/Dev/llvm-clang-3.5.0-macosx-apple-darwin/lib/clang/3.5.0/lib/darwin/libclang_rt.asan_osx_dynamic.dylib+0x2fdba)
    #1 0x80bbe2 in opj_dwt_decode_tile /Users/Matt/Dev/OpenJpeg/issue391/src/lib/openjp2/dwt.c:572:2
    #2 0x80b9e0 in opj_dwt_decode /Users/Matt/Dev/OpenJpeg/issue391/src/lib/openjp2/dwt.c:475:9
    #3 0x87b5e7 in opj_tcd_dwt_decode /Users/Matt/Dev/OpenJpeg/issue391/src/lib/openjp2/tcd.c:1550:31
    #4 0x87b04b in opj_tcd_decode_tile /Users/Matt/Dev/OpenJpeg/issue391/src/lib/openjp2/tcd.c:1242:20
    #5 0x827e17 in opj_j2k_decode_tile /Users/Matt/Dev/OpenJpeg/issue391/src/lib/openjp2/j2k.c:7796:15
    #6 0x83c597 in opj_j2k_decode_tiles /Users/Matt/Dev/OpenJpeg/issue391/src/lib/openjp2/j2k.c:9305:23
    #7 0x823f27 in opj_j2k_exec /Users/Matt/Dev/OpenJpeg/issue391/src/lib/openjp2/j2k.c:7187:41
    #8 0x82d8b3 in opj_j2k_decode /Users/Matt/Dev/OpenJpeg/issue391/src/lib/openjp2/j2k.c:9496:15
    #9 0x84ef63 in opj_decode /Users/Matt/Dev/OpenJpeg/issue391/src/lib/openjp2/openjpeg.c:412:10
    #10 0xe586c in main /Users/Matt/Dev/OpenJpeg/issue391/src/bin/jp2/opj_decompress.c:821:10
    #11 0x94511700 in start (/usr/lib/system/libdyld.dylib+0x3700)
    #12 0x4 (<unknown module>)

SUMMARY: AddressSanitizer: heap-buffer-overflow /Users/Matt/Dev/OpenJpeg/issue391/src/lib/openjp2/dwt.c:248
opj_dwt_interleave_h
Shadow bytes around the buggy address:
  0x206c0660: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x206c0670: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x206c0680: fa fa fa fa fa fa fa fa 00 00 00 00 00 00 00 00
  0x206c0690: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x206c06a0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
=>0x206c06b0: 00 00 00 00 00 00 00 00 00 00 04[fa]fa fa fa fa
  0x206c06c0: fa fa fa fa fa fa fa fa fd fd fd fd fd fd fd fd
  0x206c06d0: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
  0x206c06e0: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
  0x206c06f0: fd fd fd fd fd fd fd fd fd fd fd fa fa fa fa fa
  0x206c0700: fa fa fa fa fa fa fa fa fd fd fd fd fd fd fd fd
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
  Heap right redzone:      fb
  Freed heap region:       fd
  Stack left redzone:      f1
  Stack mid redzone:       f2
  Stack right redzone:     f3
  Stack partial redzone:   f4
  Stack after return:      f5
  Stack use after scope:   f8
  Global redzone:          f9
  Global init order:       f6
  Poisoned by user:        f7
  Container overflow:      fc
  ASan internal:           fe
==33814==ABORTING

Reported by mayeut on 2014-09-20 13:27:08


- _Attachment: [0.jp2](https://storage.googleapis.com/google-code-attachments/openjpeg/issue-399/comment-2/0.jp2)_

@gcode-importer
Copy link
Author

+ cc Bo Xu from Foxit 

... so that you can follow what happens on these issues.

Reported by detonin on 2014-09-28 21:18:38

@gcode-importer
Copy link
Author

kdu_expand -i ../../data/issue399/0.jp2 -o 0.bmp
Kakadu Core Error:
Illegal inclusion tag tree encountered while decoding a packet header.  This
problem can arise if empty packets are used (i.e., packets whose first header
bit is 0) and the value coded by the inclusion tag tree in a subsequent packet
is not exactly equal to the index of the quality layer in which each code-block
makes its first contribution.  Such an error may arise from a
mis-interpretation of the standard.  The problem may also occur as a result of
a corrupted code-stream.  Try re-opening the image with the resilient mode
enabled.

kdu_expand -i ../../data/issue399/0.jp2 -o 0.bmp -resilient

Consumed 9 tile-part(s) from a total of 9 tile(s).
Consumed 35,107 codestream bytes (excluding any file format) = 5.178310
bits/pel.
Processed using the multi-threaded environment, with
    2 parallel threads of execution

Reported by mayeut on 2014-09-30 19:54:57


- _Attachment: [0.bmp](https://storage.googleapis.com/google-code-attachments/openjpeg/issue-399/comment-4/0.bmp)_

@gcode-importer
Copy link
Author

This patch resets "resno_decoded" in tcd_init_tile.
Main COD marker has numlevels==5
Tile COD marker for tile 5 (4 zero based, but output has changed since) has numlevels==1

Trying to decode dwt for the tile with too much levels.

Feel free to correct the place where this shall be reset.

The patch has passed all tests & is OK (no regressions)
Renamed image to 0.j2k
The output image is not the same as Kakadu for tile 6 (5 zero based). This is probably
related to Kakadu warning.

./bin/opj_decompress -i ../../data/issue399/0.j2k -o 0.bmp

[INFO] Start to read j2k main header (0).
[INFO] Main header has been correctly decoded.
[INFO] No decoded area parameters, set the decoded area to the whole image
[INFO] Header of tile 1 / 9 has been read.
[INFO] Tile 1/9 has been decoded.
[INFO] Image data has been updated with tile 1.

[INFO] Header of tile 2 / 9 has been read.
[INFO] Tile 2/9 has been decoded.
[INFO] Image data has been updated with tile 2.

[INFO] Header of tile 3 / 9 has been read.
[INFO] Tile 3/9 has been decoded.
[INFO] Image data has been updated with tile 3.

[INFO] Header of tile 4 / 9 has been read.
[INFO] Tile 4/9 has been decoded.
[INFO] Image data has been updated with tile 4.

[INFO] Header of tile 5 / 9 has been read.
[INFO] Tile 5/9 has been decoded.
[INFO] Image data has been updated with tile 5.

[INFO] Header of tile 6 / 9 has been read.
[INFO] Tile 6/9 has been decoded.
[INFO] Image data has been updated with tile 6.

[INFO] Header of tile 7 / 9 has been read.
[INFO] Tile 7/9 has been decoded.
[INFO] Image data has been updated with tile 7.

[INFO] Header of tile 8 / 9 has been read.
[INFO] Tile 8/9 has been decoded.
[INFO] Image data has been updated with tile 8.

[INFO] Header of tile 9 / 9 has been read.
[INFO] Tile 9/9 has been decoded.
[INFO] Image data has been updated with tile 9.

[INFO] Generated Outfile 0.bmp


Reported by mayeut on 2014-10-09 18:05:22


- _Attachment: [issue399.patch](https://storage.googleapis.com/google-code-attachments/openjpeg/issue-399/comment-5/issue399.patch)_ - _Attachment: [0.j2k](https://storage.googleapis.com/google-code-attachments/openjpeg/issue-399/comment-5/0.j2k)_

@gcode-importer
Copy link
Author

@matthieu : is this issue verified then ?

Reported by detonin on 2014-10-22 10:44:57

@gcode-importer
Copy link
Author

@antonin,

Yes, it's been verified against the whole Test Suite.

Reported by mayeut on 2014-10-22 11:08:30

  • Status changed: Verified

@gcode-importer
Copy link
Author

Thanks. 
I'll close this issue and create a new (public) one with the very same image as a warning
should at least be issued (as in kakadu behaviour): the center of the image is corrupted
and we should inform the user about that.

Reported by detonin on 2014-10-22 13:14:58

@gcode-importer
Copy link
Author

This issue was closed by revision r2911.

Reported by detonin on 2014-10-22 13:16:58

  • Status changed: Fixed

@gcode-importer
Copy link
Author

Hi Antonin, when reviewing code with chromium security team, they raise a question about
the fix here:

Line 1063 of tcd.c: memset(p_code_block, 0, sizeof(opj_tcd_cblk_dec_t));

How do we know that p_code_block can accomodate sizeof(...) bytes?  Is there an
early check against, say, p_code_block->data_max_size?

Can you also cc [email protected] in this and future security issues? Thanks!

Reported by [email protected] on 2014-10-22 19:10:57

@gcode-importer
Copy link
Author

Reported by mayeut on 2014-10-22 21:06:16

@gcode-importer
Copy link
Author

@bo_xu,

Allocation of code blocks is done from line 900. When opj_tcd_code_block_dec_allocate
is called (line 973), it can be easily reviewed that p_code_block points to a memory
block of at least sizeof(opj_tcd_cblk_dec_t).

p_code_block->data_max_size relates to the size of allocated memory for p_code_block->data
& not p_code_block. 

All is ok here. 

Reported by mayeut on 2014-10-22 21:15:07

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants