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.

Sign up for GitHub

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

Jump to bottom

Security hole in j2k.c #392

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

Security hole in j2k.c #392

gcode-importer opened this issue Sep 17, 2014 · 14 comments
Assignees
@detonin
Labels
bug OpjVersion-2.x Priority-Critical

Comments

@gcode-importer
Copy link

Originally reported on Google Code with ID 392

 issue 414036: UNKNOWN in libc.so.6
    http://code.google.com/p/chromium/issues/detail?id=414036

Reported by detonin on 2014-09-17 09:08:51
@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 r2885

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

[INFO] Start to read j2k main header (119).
==33677==ERROR: AddressSanitizer failed to allocate 0x82105000 (-2112860160) bytes
of LargeMmapAllocator (errno: 12)
==33677==Process memory map follows:
    0x95e8a000-0x95eaf000   /usr/lib/libc++abi.dylib
    0xa102b000-0xa102c000   /usr/lib/libc++abi.dylib
    0xa59cf000-0xa8d1c000   /usr/lib/libc++abi.dylib
    0x94b8b000-0x94bb0000   /usr/lib/system/libxpc.dylib
    0xa0f04000-0xa0f06000   /usr/lib/system/libxpc.dylib
    0xa59cf000-0xa8d1c000   /usr/lib/system/libxpc.dylib
    0x98d60000-0x98d67000   /usr/lib/system/libunwind.dylib
    0xa1368000-0xa1369000   /usr/lib/system/libunwind.dylib
    0xa59cf000-0xa8d1c000   /usr/lib/system/libunwind.dylib
    0x9572a000-0x9572c000   /usr/lib/system/libunc.dylib
    0xa0fd0000-0xa0fd1000   /usr/lib/system/libunc.dylib
    0xa59cf000-0xa8d1c000   /usr/lib/system/libunc.dylib
    0x900a9000-0x900ab000   /usr/lib/system/libsystem_sandbox.dylib
    0xa0235000-0xa0236000   /usr/lib/system/libsystem_sandbox.dylib
    0xa59cf000-0xa8d1c000   /usr/lib/system/libsystem_sandbox.dylib
    0x9c21b000-0x9c223000   /usr/lib/system/libsystem_pthread.dylib
    0xa18c1000-0xa18c3000   /usr/lib/system/libsystem_pthread.dylib
    0xa59cf000-0xa8d1c000   /usr/lib/system/libsystem_pthread.dylib
    0x9b1ec000-0x9b1f2000   /usr/lib/system/libsystem_platform.dylib
    0xa1797000-0xa1798000   /usr/lib/system/libsystem_platform.dylib
    0xa59cf000-0xa8d1c000   /usr/lib/system/libsystem_platform.dylib
    0x95658000-0x95662000   /usr/lib/system/libsystem_notify.dylib
    0xa0fc5000-0xa0fc6000   /usr/lib/system/libsystem_notify.dylib
    0xa59cf000-0xa8d1c000   /usr/lib/system/libsystem_notify.dylib
    0x9bd44000-0x9bd70000   /usr/lib/system/libsystem_network.dylib
    0xa187b000-0xa187d000   /usr/lib/system/libsystem_network.dylib
    0xa187d000-0xa187e000   /usr/lib/system/libsystem_network.dylib
    0xa59cf000-0xa8d1c000   /usr/lib/system/libsystem_network.dylib
    0x98d47000-0x98d60000   /usr/lib/system/libsystem_malloc.dylib
    0xa1367000-0xa1368000   /usr/lib/system/libsystem_malloc.dylib
    0xa59cf000-0xa8d1c000   /usr/lib/system/libsystem_malloc.dylib
    0x9054b000-0x9057d000   /usr/lib/system/libsystem_m.dylib
    0xa025d000-0xa025e000   /usr/lib/system/libsystem_m.dylib
    0xa59cf000-0xa8d1c000   /usr/lib/system/libsystem_m.dylib
    0x9bc3f000-0x9bc5d000   /usr/lib/system/libsystem_kernel.dylib
    0xa1842000-0xa1844000   /usr/lib/system/libsystem_kernel.dylib
    0xa59cf000-0xa8d1c000   /usr/lib/system/libsystem_kernel.dylib
    0x9927e000-0x992a7000   /usr/lib/system/libsystem_info.dylib
    0xa13cb000-0xa13cd000   /usr/lib/system/libsystem_info.dylib
    0xa59cf000-0xa8d1c000   /usr/lib/system/libsystem_info.dylib
    0x95e62000-0x95e6b000   /usr/lib/system/libsystem_dnssd.dylib
    0xa1025000-0xa1026000   /usr/lib/system/libsystem_dnssd.dylib
    0xa59cf000-0xa8d1c000   /usr/lib/system/libsystem_dnssd.dylib
    0x98774000-0x98777000   /usr/lib/system/libsystem_configuration.dylib
    0xa12b9000-0xa12ba000   /usr/lib/system/libsystem_configuration.dylib
    0xa59cf000-0xa8d1c000   /usr/lib/system/libsystem_configuration.dylib
    0x926d5000-0x92768000   /usr/lib/system/libsystem_c.dylib
    0xa0440000-0xa0447000   /usr/lib/system/libsystem_c.dylib
    0xa59cf000-0xa8d1c000   /usr/lib/system/libsystem_c.dylib
    0x9be9f000-0x9bea1000   /usr/lib/system/libsystem_blocks.dylib
    0xa1890000-0xa1891000   /usr/lib/system/libsystem_blocks.dylib
    0xa59cf000-0xa8d1c000   /usr/lib/system/libsystem_blocks.dylib
    0x90538000-0x9054b000   /usr/lib/system/libsystem_asl.dylib
    0xa025c000-0xa025d000   /usr/lib/system/libsystem_asl.dylib
    0xa59cf000-0xa8d1c000   /usr/lib/system/libsystem_asl.dylib
    0x921f7000-0x921f9000   /usr/lib/system/libremovefile.dylib
    0xa03ec000-0xa03ed000   /usr/lib/system/libremovefile.dylib
    0xa59cf000-0xa8d1c000   /usr/lib/system/libremovefile.dylib
    0x98ef1000-0x98ef4000   /usr/lib/system/libquarantine.dylib
    0xa1378000-0xa1379000   /usr/lib/system/libquarantine.dylib
    0xa59cf000-0xa8d1c000   /usr/lib/system/libquarantine.dylib
    0x9b1f2000-0x9b1f7000   /usr/lib/system/libmacho.dylib
    0xa1798000-0xa1799000   /usr/lib/system/libmacho.dylib
    0xa59cf000-0xa8d1c000   /usr/lib/system/libmacho.dylib
    0x944e5000-0x944ee000   /usr/lib/system/liblaunch.dylib
    0xa0ece000-0xa0ecf000   /usr/lib/system/liblaunch.dylib
    0xa59cf000-0xa8d1c000   /usr/lib/system/liblaunch.dylib
    0x96c9e000-0x96c9f000   /usr/lib/system/libkeymgr.dylib
    0xa10ba000-0xa10bb000   /usr/lib/system/libkeymgr.dylib
    0xa59cf000-0xa8d1c000   /usr/lib/system/libkeymgr.dylib
    0x9450e000-0x94512000   /usr/lib/system/libdyld.dylib
    0xa0ed3000-0xa0ed4000   /usr/lib/system/libdyld.dylib
    0xa59cf000-0xa8d1c000   /usr/lib/system/libdyld.dylib
    0x9ba11000-0x9ba2a000   /usr/lib/system/libdispatch.dylib
    0xa17fb000-0xa17ff000   /usr/lib/system/libdispatch.dylib
    0xa59cf000-0xa8d1c000   /usr/lib/system/libdispatch.dylib
    0x9c008000-0x9c059000   /usr/lib/system/libcorecrypto.dylib
    0xa18a5000-0xa18a8000   /usr/lib/system/libcorecrypto.dylib
    0xa59cf000-0xa8d1c000   /usr/lib/system/libcorecrypto.dylib
    0x9ba06000-0x9ba0f000   /usr/lib/system/libcopyfile.dylib
    0xa17f8000-0xa17f9000   /usr/lib/system/libcopyfile.dylib
    0xa59cf000-0xa8d1c000   /usr/lib/system/libcopyfile.dylib
    0x9bc2e000-0x9bc34000   /usr/lib/system/libcompiler_rt.dylib
    0xa183d000-0xa183f000   /usr/lib/system/libcompiler_rt.dylib
    0xa59cf000-0xa8d1c000   /usr/lib/system/libcompiler_rt.dylib
    0x9c18b000-0x9c197000   /usr/lib/system/libcommonCrypto.dylib
    0xa18ba000-0xa18bb000   /usr/lib/system/libcommonCrypto.dylib
    0xa59cf000-0xa8d1c000   /usr/lib/system/libcommonCrypto.dylib
    0x9c197000-0x9c19c000   /usr/lib/system/libcache.dylib
    0xa18bb000-0xa18bc000   /usr/lib/system/libcache.dylib
    0xa59cf000-0xa8d1c000   /usr/lib/system/libcache.dylib
    0x97252000-0x972a8000   /usr/lib/libc++.1.dylib
    0xa1112000-0xa1118000   /usr/lib/libc++.1.dylib
    0xa59cf000-0xa8d1c000   /usr/lib/libc++.1.dylib
    0x95a22000-0x95a24000   /usr/lib/libSystem.B.dylib
    0xa0ff9000-0xa0ffa000   /usr/lib/libSystem.B.dylib
    0xa59cf000-0xa8d1c000   /usr/lib/libSystem.B.dylib
    0x007bd000-0x00846000   /Users/Matt/Dev/OpenJpeg/issue391/build/bin/libopenjp2.7.dylib
    0x00846000-0x0084d000   /Users/Matt/Dev/OpenJpeg/issue391/build/bin/libopenjp2.7.dylib
    0x0084d000-0x00867000   /Users/Matt/Dev/OpenJpeg/issue391/build/bin/libopenjp2.7.dylib
    0x002d3000-0x00330000   /Users/Matt/Dev/llvm-clang-3.5.0-macosx-apple-darwin/lib/clang/3.5.0/lib/darwin/libclang_rt.asan_osx_dynamic.dylib
    0x00330000-0x00789000   /Users/Matt/Dev/llvm-clang-3.5.0-macosx-apple-darwin/lib/clang/3.5.0/lib/darwin/libclang_rt.asan_osx_dynamic.dylib
    0x00789000-0x007bd000   /Users/Matt/Dev/llvm-clang-3.5.0-macosx-apple-darwin/lib/clang/3.5.0/lib/darwin/libclang_rt.asan_osx_dynamic.dylib
    0x00098000-0x00099000   /Users/Matt/Dev/OpenJpeg/issue391/build/./bin/opj_decompress
    0x00099000-0x00265000   /Users/Matt/Dev/OpenJpeg/issue391/build/./bin/opj_decompress
    0x00265000-0x00280000   /Users/Matt/Dev/OpenJpeg/issue391/build/./bin/opj_decompress
    0x00280000-0x002d2000   /Users/Matt/Dev/OpenJpeg/issue391/build/./bin/opj_decompress
==33677==End of process memory map.
==33677==AddressSanitizer CHECK failed: /private/tmp/llvm-release/final/llvm.src/projects/compiler-rt/lib/sanitizer_common/sanitizer_posix.cc:121
"(("unable to mmap" && 0)) != (0)" (0x0, 0x0)
    #0 0x30d227 in __asan::AsanCheckFailed(char const*, int, char const*, unsigned
long long, unsigned long long) (/Users/Matt/Dev/llvm-clang-3.5.0-macosx-apple-darwin/lib/clang/3.5.0/lib/darwin/libclang_rt.asan_osx_dynamic.dylib+0x3a227)
    #1 0x3116a3 in __sanitizer::CheckFailed(char const*, int, char const*, unsigned
long long, unsigned long long) (/Users/Matt/Dev/llvm-clang-3.5.0-macosx-apple-darwin/lib/clang/3.5.0/lib/darwin/libclang_rt.asan_osx_dynamic.dylib+0x3e6a3)

Reported by mayeut on 2014-09-20 13:17:00

- _Attachment: [0.jp2](https://storage.googleapis.com/google-code-attachments/openjpeg/issue-392/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:37

@gcode-importer
Copy link
Author 

OpenJPEG output :

$ opj_decompress.exe -i ~/data/opj/issues/issue392/0.jp2 -o ~/data/opj/issues/issue392/0.jp2_opj.pp
m

[INFO] Start to read j2k main header (119).
[ERROR] Not enough memory to increase the size of ppm_data to add the new (incomplete)
Ippm series
[ERROR] Marker handler function failed to read the marker segment
ERROR -> opj_decompress: failed to read the header


Kakadu output : 

$ kdu_expand.exe -i ~/data/opj/issues/issue392/0.jp2 -o ~/data/opj/issues/issue392/0.jp2_kdu.ppm
Kakadu Core Error:
Illegal component index supplied in call to `kdu_codesteram::get_dims'.

Reported by detonin on 2014-09-30 12:53:02

@gcode-importer
Copy link
Author 

This is better with this on x64 but still an issue with a big allocation on x86 (however
issue only under ASan as malloc shall return null on x86 systems).
Verified against test suite.

@antonin,
To get read of the extra (one might say large) allocation, we could modify opj_dec_memory_marker_handler_t
definition so that the handler (function pointer) member takes an additional parameter
(remaining bytes in the whole stream) to add a check in j2k_read_ppm_v3 & fail before
an allocation is attempted.

Reported by mayeut on 2014-10-24 21:59:45

- _Attachment: [issue392.patch](https://storage.googleapis.com/google-code-attachments/openjpeg/issue-392/comment-5/issue392.patch)_

@gcode-importer
Copy link
Author 

Under ASan :

x64
./bin/opj_decompress -i ../../data/issue392/0.jp2 -o 0.bmp

[INFO] Start to read j2k main header (119).
[ERROR] Not enough memory to increase the size of ppm_data to add the new (complete)
Ippm series
[ERROR] Marker handler function failed to read the marker segment
ERROR -> opj_decompress: failed to read the header

x86
./bin/opj_decompress -i ../../data/issue392/0.jp2 -o 0.bmp

[INFO] Start to read j2k main header (119).
==13166==ERROR: AddressSanitizer failed to allocate 0x82105000 (-2112860160) bytes
of LargeMmapAllocator (errno: 12)
==13166==Process memory map follows:
    0x9524f000-0x95274000   /usr/lib/libc++abi.dylib
    0xa090b000-0xa090c000   /usr/lib/libc++abi.dylib
    0xa59fd000-0xa8d4e000   /usr/lib/libc++abi.dylib
    0x9902b000-0x99050000   /usr/lib/system/libxpc.dylib
    0xa15b1000-0xa15b3000   /usr/lib/system/libxpc.dylib
    0xa59fd000-0xa8d4e000   /usr/lib/system/libxpc.dylib
    0x97309000-0x97310000   /usr/lib/system/libunwind.dylib
    0xa0b03000-0xa0b04000   /usr/lib/system/libunwind.dylib
    0xa59fd000-0xa8d4e000   /usr/lib/system/libunwind.dylib
    0x967b8000-0x967ba000   /usr/lib/system/libunc.dylib
    0xa0a69000-0xa0a6a000   /usr/lib/system/libunc.dylib
    0xa59fd000-0xa8d4e000   /usr/lib/system/libunc.dylib
    0x910e6000-0x910e8000   /usr/lib/system/libsystem_sandbox.dylib
    0xa03b2000-0xa03b3000   /usr/lib/system/libsystem_sandbox.dylib
    0xa59fd000-0xa8d4e000   /usr/lib/system/libsystem_sandbox.dylib
    0x9bb6e000-0x9bb76000   /usr/lib/system/libsystem_pthread.dylib
    0xa187c000-0xa187e000   /usr/lib/system/libsystem_pthread.dylib
    0xa59fd000-0xa8d4e000   /usr/lib/system/libsystem_pthread.dylib
    0x944d5000-0x944db000   /usr/lib/system/libsystem_platform.dylib
    0xa082f000-0xa0830000   /usr/lib/system/libsystem_platform.dylib
    0xa59fd000-0xa8d4e000   /usr/lib/system/libsystem_platform.dylib
    0x904c8000-0x904d2000   /usr/lib/system/libsystem_notify.dylib
    0xa026e000-0xa026f000   /usr/lib/system/libsystem_notify.dylib
    0xa59fd000-0xa8d4e000   /usr/lib/system/libsystem_notify.dylib
    0x930c7000-0x930f3000   /usr/lib/system/libsystem_network.dylib
    0xa06e6000-0xa06e8000   /usr/lib/system/libsystem_network.dylib
    0xa06e8000-0xa06e9000   /usr/lib/system/libsystem_network.dylib
    0xa59fd000-0xa8d4e000   /usr/lib/system/libsystem_network.dylib
    0x93178000-0x93191000   /usr/lib/system/libsystem_malloc.dylib
    0xa06fb000-0xa06fc000   /usr/lib/system/libsystem_malloc.dylib
    0xa59fd000-0xa8d4e000   /usr/lib/system/libsystem_malloc.dylib
    0x982f8000-0x9832a000   /usr/lib/system/libsystem_m.dylib
    0xa14a3000-0xa14a4000   /usr/lib/system/libsystem_m.dylib
    0xa59fd000-0xa8d4e000   /usr/lib/system/libsystem_m.dylib
    0x9ba7e000-0x9ba9c000   /usr/lib/system/libsystem_kernel.dylib
    0xa186d000-0xa186f000   /usr/lib/system/libsystem_kernel.dylib
    0xa59fd000-0xa8d4e000   /usr/lib/system/libsystem_kernel.dylib
    0x9bcf4000-0x9bd1d000   /usr/lib/system/libsystem_info.dylib
    0xa18a0000-0xa18a2000   /usr/lib/system/libsystem_info.dylib
    0xa59fd000-0xa8d4e000   /usr/lib/system/libsystem_info.dylib
    0x9a444000-0x9a44d000   /usr/lib/system/libsystem_dnssd.dylib
    0xa1686000-0xa1687000   /usr/lib/system/libsystem_dnssd.dylib
    0xa59fd000-0xa8d4e000   /usr/lib/system/libsystem_dnssd.dylib
    0x998d5000-0x998d8000   /usr/lib/system/libsystem_configuration.dylib
    0xa160d000-0xa160e000   /usr/lib/system/libsystem_configuration.dylib
    0xa59fd000-0xa8d4e000   /usr/lib/system/libsystem_configuration.dylib
    0x90587000-0x9061a000   /usr/lib/system/libsystem_c.dylib
    0xa0274000-0xa027b000   /usr/lib/system/libsystem_c.dylib
    0xa59fd000-0xa8d4e000   /usr/lib/system/libsystem_c.dylib
    0x95154000-0x95156000   /usr/lib/system/libsystem_blocks.dylib
    0xa08f9000-0xa08fa000   /usr/lib/system/libsystem_blocks.dylib
    0xa59fd000-0xa8d4e000   /usr/lib/system/libsystem_blocks.dylib
    0x930a9000-0x930bc000   /usr/lib/system/libsystem_asl.dylib
    0xa06e3000-0xa06e4000   /usr/lib/system/libsystem_asl.dylib
    0xa59fd000-0xa8d4e000   /usr/lib/system/libsystem_asl.dylib
    0x95432000-0x95434000   /usr/lib/system/libremovefile.dylib
    0xa093c000-0xa093d000   /usr/lib/system/libremovefile.dylib
    0xa59fd000-0xa8d4e000   /usr/lib/system/libremovefile.dylib
    0x9ba9c000-0x9ba9f000   /usr/lib/system/libquarantine.dylib
    0xa186f000-0xa1870000   /usr/lib/system/libquarantine.dylib
    0xa59fd000-0xa8d4e000   /usr/lib/system/libquarantine.dylib
    0x97681000-0x97686000   /usr/lib/system/libmacho.dylib
    0xa1374000-0xa1375000   /usr/lib/system/libmacho.dylib
    0xa59fd000-0xa8d4e000   /usr/lib/system/libmacho.dylib
    0x98f2c000-0x98f35000   /usr/lib/system/liblaunch.dylib
    0xa1596000-0xa1597000   /usr/lib/system/liblaunch.dylib
    0xa59fd000-0xa8d4e000   /usr/lib/system/liblaunch.dylib
    0x96d85000-0x96d86000   /usr/lib/system/libkeymgr.dylib
    0xa0ad7000-0xa0ad8000   /usr/lib/system/libkeymgr.dylib
    0xa59fd000-0xa8d4e000   /usr/lib/system/libkeymgr.dylib
    0x9aaee000-0x9aaf2000   /usr/lib/system/libdyld.dylib
    0xa173f000-0xa1740000   /usr/lib/system/libdyld.dylib
    0xa59fd000-0xa8d4e000   /usr/lib/system/libdyld.dylib
    0x930f5000-0x9310e000   /usr/lib/system/libdispatch.dylib
    0xa06ea000-0xa06ee000   /usr/lib/system/libdispatch.dylib
    0xa59fd000-0xa8d4e000   /usr/lib/system/libdispatch.dylib
    0x97688000-0x976d9000   /usr/lib/system/libcorecrypto.dylib
    0xa1376000-0xa1379000   /usr/lib/system/libcorecrypto.dylib
    0xa59fd000-0xa8d4e000   /usr/lib/system/libcorecrypto.dylib
    0x9b4de000-0x9b4e7000   /usr/lib/system/libcopyfile.dylib
    0xa1814000-0xa1815000   /usr/lib/system/libcopyfile.dylib
    0xa59fd000-0xa8d4e000   /usr/lib/system/libcopyfile.dylib
    0x9c0c3000-0x9c0c9000   /usr/lib/system/libcompiler_rt.dylib
    0xa18ce000-0xa18d0000   /usr/lib/system/libcompiler_rt.dylib
    0xa59fd000-0xa8d4e000   /usr/lib/system/libcompiler_rt.dylib
    0x90008000-0x90014000   /usr/lib/system/libcommonCrypto.dylib
    0xa0252000-0xa0253000   /usr/lib/system/libcommonCrypto.dylib
    0xa59fd000-0xa8d4e000   /usr/lib/system/libcommonCrypto.dylib
    0x9c1ad000-0x9c1b2000   /usr/lib/system/libcache.dylib
    0xa18e2000-0xa18e3000   /usr/lib/system/libcache.dylib
    0xa59fd000-0xa8d4e000   /usr/lib/system/libcache.dylib
    0x9a905000-0x9a95b000   /usr/lib/libc++.1.dylib
    0xa170e000-0xa1714000   /usr/lib/libc++.1.dylib
    0xa59fd000-0xa8d4e000   /usr/lib/libc++.1.dylib
    0x930f3000-0x930f5000   /usr/lib/libSystem.B.dylib
    0xa06e9000-0xa06ea000   /usr/lib/libSystem.B.dylib
    0xa59fd000-0xa8d4e000   /usr/lib/libSystem.B.dylib
    0x007a1000-0x0082c000   /Users/Matt/Dev/OpenJpeg/issue398/build/bin/libopenjp2.7.dylib
    0x0082c000-0x00833000   /Users/Matt/Dev/OpenJpeg/issue398/build/bin/libopenjp2.7.dylib
    0x00833000-0x0084d000   /Users/Matt/Dev/OpenJpeg/issue398/build/bin/libopenjp2.7.dylib
    0x002b6000-0x00313000   /Users/Matt/Dev/llvm-clang-3.5.0-macosx-apple-darwin/lib/clang/3.5.0/lib/darwin/libclang_rt.asan_osx_dynamic.dylib
    0x00313000-0x0076c000   /Users/Matt/Dev/llvm-clang-3.5.0-macosx-apple-darwin/lib/clang/3.5.0/lib/darwin/libclang_rt.asan_osx_dynamic.dylib
    0x0076c000-0x007a0000   /Users/Matt/Dev/llvm-clang-3.5.0-macosx-apple-darwin/lib/clang/3.5.0/lib/darwin/libclang_rt.asan_osx_dynamic.dylib
    0x0007a000-0x0007b000   /Users/Matt/Dev/OpenJpeg/issue398/build/./bin/opj_decompress
    0x0007b000-0x00248000   /Users/Matt/Dev/OpenJpeg/issue398/build/./bin/opj_decompress
    0x00248000-0x00263000   /Users/Matt/Dev/OpenJpeg/issue398/build/./bin/opj_decompress
    0x00263000-0x002b5000   /Users/Matt/Dev/OpenJpeg/issue398/build/./bin/opj_decompress
==13166==End of process memory map.
==13166==AddressSanitizer CHECK failed: /private/tmp/llvm-release/final/llvm.src/projects/compiler-rt/lib/sanitizer_common/sanitizer_posix.cc:121
"(("unable to mmap" && 0)) != (0)" (0x0, 0x0)
    #0 0x2f0227 (/Users/Matt/Dev/llvm-clang-3.5.0-macosx-apple-darwin/lib/clang/3.5.0/lib/darwin/libclang_rt.asan_osx_dynamic.dylib+0x3a227)
    #1 0x2f46a3 (/Users/Matt/Dev/llvm-clang-3.5.0-macosx-apple-darwin/lib/clang/3.5.0/lib/darwin/libclang_rt.asan_osx_dynamic.dylib+0x3e6a3)

Reported by mayeut on 2014-10-24 22:01:08

@gcode-importer
Copy link
Author 

@antonin, any update on this issue?

Reported by bo_xu@foxitsoftware.com on 2014-10-29 17:06:02

@gcode-importer
Copy link
Author 

This issue was updated by revision r2919.


Issue left open to solve problem on x64-Asan and to investigate Matthieu's suggestion

Reported by detonin on 2014-10-30 11:53:12

@gcode-importer
Copy link
Author 

@antonin,

I added a check on stream length :

x64
./bin/opj_decompress -i ../../data/issue392/0.jp2 -o 0.bmp

[INFO] Start to read j2k main header (119).
[ERROR] Not enough bytes (476 + 279389) to hold Ippm series (2182094848), Index (6)
[ERROR] Marker handler function failed to read the marker segment
ERROR -> opj_decompress: failed to read the header

x86
./bin/opj_decompress -i ../../data/issue392/0.jp2 -o 0.bmp

[INFO] Start to read j2k main header (119).
[ERROR] Not enough bytes (476 + 279389) to hold Ippm series (2182094848), Index (6)
[ERROR] Marker handler function failed to read the marker segment
ERROR -> opj_decompress: failed to read the header

No regressions against the whole test suite.

Reported by mayeut on 2014-11-04 21:02:08

  • Status changed: Verified
- _Attachment: [issue392-part2.patch](https://storage.googleapis.com/google-code-attachments/openjpeg/issue-392/comment-9/issue392-part2.patch)_

@gcode-importer
Copy link
Author 

@matthieu: thanks for the part-2 of the patch. Instead of adding a new parameter stream_size
(which should actually be renamed p_stream_size to remain consistent with naming convention),
I think the first part of the test is enough : 

if (l_N_ppm > p_header_size) {

instead of 

if ((l_N_ppm > p_header_size) && (((OPJ_OFF_T)(l_N_ppm - p_header_size)) > stream_size))
{

as Ippm should be included in the header. Is there a reason why you want to compare
to remaining stream size ?

Thanks

Reported by detonin on 2014-12-09 13:55:45

@gcode-importer
Copy link
Author 

@antonin,

The first part is not enough because incomplete Ippm series are allowed (this is what
I understand).

We can live without anymore checks. As I said in comment #5, this is to fail earlier
than now (to get read of the extra allocation which will fail & makes ASan to report
an error).

The check compares if l_N_ppm bytes can exist within p_header_size + stream_size. If
not, we won't be able to read the rest of the incomplete Ippm series in the next ppm
marker so fail early without trying to allocate memory.

Reported by mayeut on 2014-12-09 14:27:14

@gcode-importer
Copy link
Author 

@matthieu,

thanks, you're right, Ippm series might be incomplete. Then I assume following sanity
checks : 
https://code.google.com/p/openjpeg/source/browse/trunk/src/lib/openjp2/j2k.c#3585
https://code.google.com/p/openjpeg/source/browse/trunk/src/lib/openjp2/j2k.c#3629
are wrong too and should be removed. 

Moreover, I see that there is no check on the Zppm value (except for the very first
one which should be 0). My feeling is that this whole function j2k_read_ppm_v3 should
be re-written (and renamed actually) in the following way : while reading the MH (in
opj_j2k_read_header_procedure), concatenate all Nppm and corresponding Ippm's and parse
them all at once, once getting out of the while loop of the MH processing. This would
allow the processing of ppm data in the right order (if by any chance Zppm are not
in increasing order) and would avoid the need of adding a parameter to all marker handlers.
What do you think ? I would create a new issue for this.

Reported by detonin on 2014-12-09 18:18:54

@gcode-importer
Copy link
Author 

@antonin,

You're probably right regarding those checks. Reading the code I thought that the first
Nppm,Ippm in the marker shall be complete with remaining Nppm,Ippm possibly incomplete
(This is exactly what those checks are doing). Reading the standard, I can't find anything
that support this theory but maybe I'm wrong.
You're right about Zppm.

As I said earlier, this added check is not necessary to close this issue. It would
have been best not to have the ASan error but there's no security issue anymore.
I guess that creating a new issue as you suggest might be a good option.

Reported by mayeut on 2014-12-09 18:43:55

@gcode-importer
Copy link
Author 

new issue 470 created to refactor j2k_read_ppm_v3.

I close this issue as there is no security issue anymore.

Reported by detonin on 2015-01-15 17:41:11

  • Status changed: Fixed
  • Labels removed: Restrict-View-CoreTeam

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@detonin detonin

Labels
bug OpjVersion-2.x Priority-Critical
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@detonin @gcode-importer