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_t1_decode_cblks #432

Closed
gcode-importer opened this issue Nov 14, 2014 · 10 comments
Closed

heap-buffer-overflow in opj_t1_decode_cblks #432

gcode-importer opened this issue Nov 14, 2014 · 10 comments

Comments

@gcode-importer
Copy link

Originally reported on Google Code with ID 432

https://code.google.com/p/chromium/issues/detail?id=429139

Reproduced on trunk r2924 MacOS x64 ASan build :

./bin/opj_decompress -i ../../ex/1.jp2 -o 0.bmp

[INFO] Start to read j2k main header (129).
[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 / 3 has been read.
=================================================================
==3446==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x000117752914 at
pc 0x00010cb723b5 bp 0x7fff53fe4040 sp 0x7fff53fe4038
WRITE of size 4 at 0x000117752914 thread T0
    #0 0x10cb723b4 in opj_t1_decode_cblks /Users/Matt/Dev/OpenJpeg/issue/src/lib/openjp2/t1.c:1346:33
    #1 0x10cb8b1fe in opj_tcd_t1_decode /Users/Matt/Dev/OpenJpeg/issue/src/lib/openjp2/tcd.c:1537:34
    #2 0x10cb8af36 in opj_tcd_decode_tile /Users/Matt/Dev/OpenJpeg/issue/src/lib/openjp2/tcd.c:1253:20
    #3 0x10cb3c48d in opj_j2k_decode_tile /Users/Matt/Dev/OpenJpeg/issue/src/lib/openjp2/j2k.c:7916:15
    #4 0x10cb4f0b1 in opj_j2k_decode_tiles /Users/Matt/Dev/OpenJpeg/issue/src/lib/openjp2/j2k.c:9430:23
    #5 0x10cb38daa in opj_j2k_exec /Users/Matt/Dev/OpenJpeg/issue/src/lib/openjp2/j2k.c:7294:41
    #6 0x10cb41101 in opj_j2k_decode /Users/Matt/Dev/OpenJpeg/issue/src/lib/openjp2/j2k.c:9621:15
    #7 0x10cb55ffd in opj_jp2_decode /Users/Matt/Dev/OpenJpeg/issue/src/lib/openjp2/jp2.c:1398:8
    #8 0x10bc1c944 in main /Users/Matt/Dev/OpenJpeg/issue/src/bin/jp2/opj_decompress.c:821:10
    #9 0x7fff8f0735c8 in start (/usr/lib/system/libdyld.dylib+0x35c8)
    #10 0x4 (<unknown module>)

0x000117752914 is located 1044 bytes to the right of 141032704-byte region [0x00010f0d2800,0x000117752500)
allocated by thread T0 here:
    #0 0x10be79ccf 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+0x2bccf)
    #1 0x10cb83ceb in opj_alloc_tile_component_data /Users/Matt/Dev/OpenJpeg/issue/src/lib/openjp2/tcd.c:617:33
    #2 0x10cb87386 in opj_tcd_init_decode_tile /Users/Matt/Dev/OpenJpeg/issue/src/lib/openjp2/tcd.c:1012:1
    #3 0x10cb3ab71 in opj_j2k_read_tile_header /Users/Matt/Dev/OpenJpeg/issue/src/lib/openjp2/j2k.c:7867:15
    #4 0x10cb4ef87 in opj_j2k_decode_tiles /Users/Matt/Dev/OpenJpeg/issue/src/lib/openjp2/j2k.c:9402:23
    #5 0x10cb38daa in opj_j2k_exec /Users/Matt/Dev/OpenJpeg/issue/src/lib/openjp2/j2k.c:7294:41
    #6 0x10cb41101 in opj_j2k_decode /Users/Matt/Dev/OpenJpeg/issue/src/lib/openjp2/j2k.c:9621:15
    #7 0x10cb55ffd in opj_jp2_decode /Users/Matt/Dev/OpenJpeg/issue/src/lib/openjp2/jp2.c:1398:8
    #8 0x10bc1c944 in main /Users/Matt/Dev/OpenJpeg/issue/src/bin/jp2/opj_decompress.c:821:10
    #9 0x7fff8f0735c8 in start (/usr/lib/system/libdyld.dylib+0x35c8)
    #10 0x4 (<unknown module>)

SUMMARY: AddressSanitizer: heap-buffer-overflow /Users/Matt/Dev/OpenJpeg/issue/src/lib/openjp2/t1.c:1346
opj_t1_decode_cblks
Shadow bytes around the buggy address:
  0x100022eea4d0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x100022eea4e0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x100022eea4f0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x100022eea500: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x100022eea510: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
=>0x100022eea520: fa fa[fa]fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x100022eea530: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x100022eea540: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x100022eea550: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x100022eea560: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x100022eea570: 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
  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
==3446==ABORTING

Reported by mayeut on 2014-11-14 21:14:40

@gcode-importer
Copy link
Author

Reported by mayeut on 2014-11-14 21:15:15


- _Attachment: [issue432.jp2](https://storage.googleapis.com/google-code-attachments/openjpeg/issue-432/comment-1/issue432.jp2)_

@gcode-importer
Copy link
Author

kdu_expand -i ../../ex/issue432.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.

Reported by mayeut on 2014-11-20 22:41:52

@gcode-importer
Copy link
Author

This issue was updated by revision r2937.

Reported by mayeut on 2014-11-21 21:34:30

  • Status changed: Started

@gcode-importer
Copy link
Author

This issue was closed by revision r2938.

Reported by mayeut on 2014-11-21 21:35:57

  • Status changed: Fixed

@gcode-importer gcode-importer self-assigned this Jun 11, 2015
stweil added a commit to stweil/openjpeg that referenced this issue Mar 25, 2016
The maximum supported data size is a limiting factor for the maximum
size of allowed images. As it is possible to allocate size_t bytes,
this data type must also be used for the data size.

This modification also fixes issue uclouvain#432. That file is now decoded on
64 bit hosts with enough RAM, so the regression test had to be fixed.

Update also some comments in the test suite.

Signed-off-by: Stefan Weil <sw@weilnetz.de>
stweil added a commit to stweil/openjpeg that referenced this issue Mar 25, 2016
The maximum supported data size is a limiting factor for the maximum
size of allowed images. As it is possible to allocate size_t bytes,
this data type must also be used for the data size.

This modification also fixes issue uclouvain#432. That file is now decoded on
64 bit hosts with enough RAM, so the regression test had to be fixed.
Until it is possible to use different test cases for 32 and 64 bit hosts,
the test for issue uclouvain#432 is disabled.

Update also some comments in the test suite.

Signed-off-by: Stefan Weil <sw@weilnetz.de>
stweil added a commit to stweil/openjpeg that referenced this issue Mar 28, 2016
The maximum supported data size is a limiting factor for the maximum
size of allowed images. As it is possible to allocate size_t bytes,
this data type must also be used for the data size.

This modification also fixes issue uclouvain#432. That file is now decoded on
64 bit hosts with enough RAM, so the regression test had to be fixed.
Until it is possible to use different test cases for 32 and 64 bit hosts,
the test for issue uclouvain#432 is disabled.

Handle also a potential division by zero when l_data_size is 0.
This fixes issue uclouvain#733.

Update also some comments in the test suite.

Signed-off-by: Stefan Weil <sw@weilnetz.de>
stweil added a commit to stweil/openjpeg that referenced this issue May 8, 2016
The maximum supported data size is a limiting factor for the maximum
size of allowed images. As it is possible to allocate size_t bytes,
this data type must also be used for the data size.

This modification also fixes issue uclouvain#432. That file is now decoded on
64 bit hosts with enough RAM, so the regression test had to be fixed.
Until it is possible to use different test cases for 32 and 64 bit hosts,
the test for issue uclouvain#432 is disabled.

Handle also a potential division by zero when l_data_size is 0.
This fixes issue uclouvain#733.

Update also some comments in the test suite.

Signed-off-by: Stefan Weil <sw@weilnetz.de>
stweil added a commit to stweil/openjpeg that referenced this issue May 3, 2017
The maximum supported data size is a limiting factor for the maximum
size of allowed images. As it is possible to allocate size_t bytes,
this data type must also be used for the data size.

This modification also fixes issue uclouvain#432. That file is now decoded on
64 bit hosts with enough RAM, so the regression test had to be fixed.
Until it is possible to use different test cases for 32 and 64 bit hosts,
the test for issue uclouvain#432 is disabled.

Handle also a potential division by zero when l_data_size is 0.
This fixes issue uclouvain#733.

Update also some comments in the test suite.

Signed-off-by: Stefan Weil <sw@weilnetz.de>
stweil added a commit to stweil/openjpeg that referenced this issue Aug 12, 2017
The maximum supported data size is a limiting factor for the maximum
size of allowed images. As it is possible to allocate size_t bytes,
this data type must also be used for the data size.

This modification also fixes issue uclouvain#432. That file is now decoded on
64 bit hosts with enough RAM, so the regression test had to be fixed.
Until it is possible to use different test cases for 32 and 64 bit hosts,
the test for issue uclouvain#432 is disabled.

Handle also a potential division by zero when l_data_size is 0.
This fixes issue uclouvain#733.

Update also some comments in the test suite.

Signed-off-by: Stefan Weil <sw@weilnetz.de>
stweil added a commit to stweil/openjpeg that referenced this issue Aug 24, 2017
The maximum supported data size is a limiting factor for the maximum
size of allowed images. As it is possible to allocate size_t bytes,
this data type must also be used for the data size.

This modification also fixes issue uclouvain#432. That file is now decoded on
64 bit hosts with enough RAM, so the regression test had to be fixed.
Until it is possible to use different test cases for 32 and 64 bit hosts,
the test for issue uclouvain#432 is disabled.

Handle also a potential division by zero when l_data_size is 0.
This fixes issue uclouvain#733.

Update also some comments in the test suite.

Signed-off-by: Stefan Weil <sw@weilnetz.de>
@stweil
Copy link
Contributor

stweil commented Sep 8, 2017

@rouault, is this issue fixed with your latest commits (PR #1010)?

stweil added a commit to stweil/openjpeg that referenced this issue Sep 8, 2017
The maximum supported data size is a limiting factor for the maximum
size of allowed images. As it is possible to allocate size_t bytes,
this data type must also be used for the data size.

This modification also fixes issue uclouvain#432. That file is now decoded on
64 bit hosts with enough RAM, so the regression test had to be fixed.
Until it is possible to use different test cases for 32 and 64 bit hosts,
the test for issue uclouvain#432 is disabled.

Handle also a potential division by zero when l_data_size is 0.
This fixes issue uclouvain#733.

Update also some comments in the test suite.

Signed-off-by: Stefan Weil <sw@weilnetz.de>
@rouault
Copy link
Collaborator

rouault commented Sep 8, 2017

Yes this is solved by PR #1010 (I had to extract the codestream from the jp2 container since the lib now rejects this image because of inconsistancies between IHDR and SIZ)

@stweil
Copy link
Contributor

stweil commented Sep 8, 2017

So the test passes successfully and the issue can be closed?

@rouault
Copy link
Collaborator

rouault commented Sep 8, 2017

The issue is closed since 2015 :-) The test related to 432 fails (expected fail) because of the inconsistancy between IHDR and SIZ. If we extract just the codestream, the test could perhaps on 64bit provided that there is enough memory (but my 16 GB are apparently not enough)

@stweil
Copy link
Contributor

stweil commented Sep 8, 2017

Strange. The test had passed on 64 bit with my PR.

@rouault
Copy link
Collaborator

rouault commented Sep 8, 2017

The image dimension is 1109 x 2098808 and the tile size is 1000000 x 1000000, which clamped to the image is 1109 x 1000000
I've instrumented the memory allocations in master , and they are consistant with the theory of the code:

  • 1109 * 1000000 * sizeof(uint32) = 4.4 GB for the tile component buffer
  • 1109 * 2098808 * sizeof(uint32) = 9.3 GB for the final image

So 13.7 GB in total. So a bit too high for my 16 GB machine given what is used besides
With older versions, the memory needs are even higher.

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

3 participants