[Bug] Protential stack-buffer-overflow crash in opj_compress.c #1610
Description
Hi team! Thanks for your great work!
I think I found a small vulnerability that might lead to crash in the system.
It locates at the parse_cmdline_encoder function in src/bin/jp2/opj_compress.c. The vulnerability occurs because the command-line argument parser uses sprintf at line 712 to copy a user-controlled string into a fixed-size stack buffer char outformat[50]; at line 710 without bounds checking, causing a stack buffer overflow.
PoC
we can run
./bin/opj_compress -OutFor $(python3 -c "print('A'*80)")
and we can see the following crash
=================================================================
==2036569==ERROR: AddressSanitizer: stack-buffer-overflow on address 0x7ffcf77a7752 at pc 0x7a9a2a85f04f bp 0x7ffcf77a7040 sp 0x7ffcf77a67d0
WRITE of size 82 at 0x7ffcf77a7752 thread T0
#0 0x7a9a2a85f04e in __interceptor_vsprintf ../../../../src/libsanitizer/sanitizer_common/sanitizer_common_interceptors.inc:1687
#1 0x7a9a2a85f27e in __interceptor_sprintf ../../../../src/libsanitizer/sanitizer_common/sanitizer_common_interceptors.inc:1730
#2 0x5d0829ad80c8 in parse_cmdline_encoder /openjpeg/src/bin/jp2/opj_compress.c:712
#3 0x5d0829ade660 in main /openjpeg/src/bin/jp2/opj_compress.c:1970
#4 0x7a9a2a229d8f in __libc_start_call_main ../sysdeps/nptl/libc_start_call_main.h:58
#5 0x7a9a2a229e3f in __libc_start_main_impl ../csu/libc-start.c:392
#6 0x5d0829ad3ba4 in _start (/openjpeg/build-asan/bin/opj_compress+0xbba4)Root Causes
The crash is caused by the handling of the 'O' case (associated with -OutFor option). A fixed-size stack buffer outformat of 50 bytes is declared, but sprintf is used to append the user-provided string of (from opj_optarg) to it without verifying if the length exceeds the buffer size.
case 'O': { /* output format */
char outformat[50]; // Fixed 50-byte stack buffer
char *of = opj_optarg;
sprintf(outformat, ".%s", of); // Unbounded write if 'of' > 48 chars
img_fol->set_out_format = 1;
parameters->cod_format = get_file_format(outformat);
// ...Fix
I've checked the file and I think replacing sprintf with snprintf to limit the number of bytes written would be the safest fix. Alternatively, checking the length of of before writing to the buffer would also prevent the overflow.