I reviewed openjpeg version 1:1.5.2-3.1 as checked into Xenial. This should not be considered a full security audit.
Security team NAK for moving openjpeg to main for 16.04. The source is nearly unreadable due to terrible formatting. I could not find any tab-sizes that allowed the code to be legible by any reasonable standard. I strongly recommend upstream should (a) run the entire codebase through indent(1) or a similar tool (b) enforce a standard, any standard. K&R recommended but really anything beats what this currently has. In one day of fuzzing with my laptop I found eight crashers that caused segmentation violation faults. They may be read-only or perhaps they may allow remote code execution but the source code is so difficult to read that I cannot volunteer my time to discover the causes. cppcheck as packaged with 14.04 LTS issued 21 warnings; almost all represent real bugs in the package. Upstream's 2.1.0 release is a mixed bag -- cppcheck reported 61 warnings; many looked similar to issues from 1.5.2. However, the eight crashing inputs I found no longer crashed the library. Less than four hours of fuzzing on my laptop found an input that crashes. Memory management is sloppy and may in fact provide routes for exploits. Integer types seem very casual; this library was not written with a strong awareness of C's extremely dangerous behaviours around signed integers, the usual integer promotion rules, and sign extension rules. In addition, this looks like a poor library interface -- errors are sent directly to stderr without any attempts to offer client programs any mediation, the error messages never include strerror() reasons meaning users have no feedback on what failed. We cannot support this library as it stands. Before we can support it, the source code layout needs to be addressed, cppcheck should be clean, and gcc warnings should be addressed. I strongly recommend sending this through Coverity as well. Here's a more concrete list of issues I found; I hope these are useful: - applications/codec/j2k_dump.c allocates a hugely oversized block of memory for getting directory listings. If the dirptr or dirptr->filename memory allocations fail, the program dies via NULL pointer write errors. This should instead work on one file at a time and avoid the entire disaster. - applications/codec/j2k_dump.c parameters.outfile misleading error message, the fopen() was for writing, not reading - applications/JavaOpenJPEG/JavaOpenJPEGDecoder.c get_num_images(), load_images() leak dir - Java_org_openJpeg_OpenJPEGJavaDecoder_internalDecodeJ2KtoImage() fails to close 'fsrc' in this error path: if(src == NULL) goto fin - OPJMarkerTree::OnSelChanged() allocates buffer with new[] but deallocates with delete - bmptoimage() never checks getc() error return code - bmptoimage() leaks IN - imagetopgx() leaks name via if (!fdest) error -- memory handling here is far too complicated - applications/codec/image_to_j2k.c main() far too complicated - applications/codec/image_to_j2k.c main() leaks f (not really relevant since the 'return 1' exits, but once this code is broken apart into something legible this will matter more) - yuv_num_frames() leaks f via if (end_of_f < frame_size) error - applications/mj2/mj2_to_frames.c main() far too complicated; leaks frame_codestream in the event of realloc() failure - xml_write_mdia() writes garbage via fprintf() with too few arguments - test_image() calls opj_image_destroy(image); on an image without a valid value via if(cio == NULL) error -- note doubled if(image == NULL) goto fin; checks as well, looks like copy-and-paste errors in this function too - read_siz_marker() does no fread()-related error checking - read_siz_marker() left-shifts plain 'char' values, on platforms with default signed chars this will probably cause a hugely negative value, which when passed to malloc(size_t) will be turned into a hugely positive value - applications/mj2/wrap_j2k_in_mj2.c main() far too complicated - applications/mj2/wrap_j2k_in_mj2.c main() calls free(buf) with undefined 'buf' value via if(cinfo == NULL) error path Notes to the future: Find a jpeg2000 image somewhere (I'll include one in my tarball of crashers) and stick it in fuzz/in/ ccmake to change the CC to /usr/bin/afl-gcc Build with AFL_HARDEN=1 make mkdir -p fuzz/in fuzz/out cd fuzz afl -i in -o out -f foo.j2k ../bin/<whatever the executable is> -i @@ -o foo.bmp Thanks -- You received this bug notification because you are a member of Ubuntu Touch seeded packages, which is subscribed to openjpeg in Ubuntu. https://bugs.launchpad.net/bugs/711061 Title: [MIR] openjpeg Status in openjpeg package in Ubuntu: Confirmed Bug description: libopenjpeg should be included in main because compiling poppler with --enable-openjpeg in debian/rules gives poppler greater functionality (please see bug 710412). Since this change to /debian/rules adds libopenjpeg as a build-dep to poppler, which is in main, libopenjpeg must also be in main. Main inclusion requirements: 1. It is already in the universe. 2. The package is a new build-dep, and has a large user base (think evince). 3. Searching http://secunia.com/advisories/search/ for libopenjpeg gave zero results. 4. Libopenjpeg has no current Ubuntu bugs (https://bugs.launchpad.net/ubuntu/maverick/+source/openjpeg) in the Debian bug tracking system libopenjpeg has 1 open bug (http://bugs.debian.org/cgi-bin/pkgreport.cgi?pkg=libopenjpeg2), this is an encoding bug, but the main use for this package will be decoding. Libopenjpeg does not require any configuration or debconf questions. 5. N/A 6. All build-deps are already included in main. 7. I am afraid that this is a bit over my head, hopefully someone else could ensure that this package meets the requirments here. Based on its long inclusion in Debian and Ubuntu I think that it should be alright here. 8.This is a fairly simple program not needed too much maintenance, as shown by the bug reports. 9. The package title and description seem to be in order. My only final comments are that I am sorry this may not be quite the normal MIR, but I am just a member of bug control, not a dev. Also, any help and advise along the way would be much appreciated. To manage notifications about this bug go to: https://bugs.launchpad.net/ubuntu/+source/openjpeg/+bug/711061/+subscriptions -- Mailing list: https://launchpad.net/~touch-packages Post to : [email protected] Unsubscribe : https://launchpad.net/~touch-packages More help : https://help.launchpad.net/ListHelp
