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/ -i @@ -o foo.bmp Thanks