Skip to content

openjpip: bound query parser field copies - #1656

Draft
Alb3e3 wants to merge 1 commit into
uclouvain:masterfrom
Alb3e3:alb3e3/openjpeg-jpip-query-bounds
Draft

openjpip: bound query parser field copies#1656
Alb3e3 wants to merge 1 commit into
uclouvain:masterfrom
Alb3e3:alb3e3/openjpeg-jpip-query-bounds

Conversation

@Alb3e3

@Alb3e3 Alb3e3 commented Jul 9, 2026

Copy link
Copy Markdown

Summary

  • Bound JPIP query field-name and field-value copies before writing into the fixed parse_query() stack buffers.
  • Bound metareq req-box-prop fragments before writing into the fixed parse_metareq() stack buffer, stop malformed bracket syntax, and reject requests with more than MAX_NUMOFBOX box properties before mutating query state.
  • Add a BUILD_JPIP unit test covering oversized field names, field values, both metareq fragment copy sites, and oversized metareq property lists.

Root Cause

get_fieldparam() and parse_metareq() used strncpy() lengths derived directly from attacker-controlled query delimiters. In release builds the nearby assert() checks are compiled out, so oversized JPIP query fields can write beyond fixed stack buffers. parse_metareq() also trusted the number of requested box properties and could drive parse_req_box_prop() past the MAX_NUMOFBOX arrays.

Fixes #1635.
Fixes #1636.

Validation

Red checks before the relevant fixes:

  • Oversized field-name case: ASAN_OPTIONS=detect_leaks=0:abort_on_error=1 UBSAN_OPTIONS=halt_on_error=1:abort_on_error=1 build-asan/bin/testqueryparser reported stack-buffer-overflow with WRITE of size 200.
  • Too-many metareq properties case: UBSan reported index 10 out of bounds for type 'OPJ_BOOL[10]' in query_parser.c.

Green checks after the fix:

  • cmake -S . -B build-asan -DBUILD_TESTING=ON -DBUILD_JPIP=ON -DBUILD_SHARED_LIBS=OFF -DCMAKE_BUILD_TYPE=RelWithDebInfo -DCMAKE_C_COMPILER=clang-18 -DCMAKE_C_FLAGS_RELWITHDEBINFO="-O1 -g -DNDEBUG -fsanitize=address,undefined -fno-omit-frame-pointer" -DCMAKE_EXE_LINKER_FLAGS="-fsanitize=address,undefined"
  • cmake --build build-asan --target testqueryparser -j2
  • ASAN_OPTIONS=detect_leaks=0:abort_on_error=1 UBSAN_OPTIONS=halt_on_error=1:abort_on_error=1 ctest --test-dir build-asan -R '^testqueryparser$' --output-on-failure
  • git diff --check HEAD~1..HEAD

Note: a broader sanitizer run including existing testempty1 aborts on an unrelated UBSan function-pointer type mismatch in src/lib/openjp2/openjpeg.c; the new JPIP parser regression passes cleanly.

@Alb3e3
Alb3e3 force-pushed the alb3e3/openjpeg-jpip-query-bounds branch from 7c6cc50 to 91677a6 Compare July 9, 2026 16:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[SECURITY] Stack-based buffer overflow in parse_metareq() — query_parser.c:345 Stack Buffer Overflow in OpenJPIP get_fieldparam()

1 participant