fix: reject zero-progress packet headers for CVE-2023-39327

[jeffspahr]

Addresses CVE-2023-39327.

PR #1547 requires EPH markers and fixes the exact issue1472 PoC, but malformed variants derived from the same codestream can still reach the empty-packet path with zero forward progress. With Scod = 0x03 (PRT+SOP), the decoder loops printing Not enough space for expected SOP marker. With Scod = 0x01 (PRT only), it can spend excessive time walking packet iteration after EOF.

The root cause is that opj_t2_read_packet_header() can still report a successful empty packet after consuming zero bytes. When that happens, the caller keeps iterating packets against unchanged input.

This change makes packet progression explicit in the T2 decode path. A zero-progress empty-packet header is treated as packet-header exhaustion instead of a successful packet, and the decoder finalizes resno_decoded the same way the tolerant EOF path would have ended before stopping packet iteration. This stops the malformed inputs promptly while preserving tolerated truncated decodes, including codestreams where SOP markers are allowed by Scod but absent.

It also expands the focused unit coverage to exercise three nearby malformed variants derived from the same in-memory codestream:

• Scod = 0x03 (PRT+SOP)
• Scod = 0x01 (PRT only)
• Scod = 0x07 (PRT+SOP+EPH)

The unit test also covers a tolerated EOF fixture from openjpeg-data after setting the SOP bit in Scod without adding SOP markers, so optional SOP absence remains non-fatal.

To keep the same-CVE fuzz follow-up in this PR, tests/fuzzers/build_seed_corpus.sh now derives the 0x03 and 0x01 issue1472 variants from the existing upstream issue1472-bigloop.j2k seed when building opj_decompress_fuzzer_seed_corpus.zip. The script verifies the source seed still has the expected Scod = 0x07 byte before mutating it.

Testing

• Built opj_decompress and testissue1472_noeph
• Ran ctest -R "^testissue1472_noeph$" --output-on-failure
• Reproduced the malformed Scod = 0x07 variant: it fails quickly with Not enough space for required EPH marker
• Reproduced the malformed Scod = 0x03 variant: it returns promptly with Stream too short, expected SOT
• Reproduced the malformed Scod = 0x01 variant: it returns promptly with Stream too short, expected SOT
• Rechecked the previously regressed nonregression inputs locally:
  • Marrin.jp2
  • dwt_interleave_h.gsr105.jp2
  • dwt_interleave_h.gsr105.jp2 -d 1,1,33,33
  • issue226.j2k (same local baseline failure as master)
• Rechecked SOP-bit/no-SOP tolerated EOF variants of:
  • Marrin.jp2
  • dwt_interleave_h.gsr105.jp2
• Ran tests/fuzzers/build_seed_corpus.sh against the standard openjpeg-data checkout and confirmed opj_decompress_fuzzer_seed_corpus.zip contains:
  • issue1472-bigloop.j2k
  • issue1472-bigloop-noeph.j2k
  • issue1472-bigloop-prt-only.j2k

[rouault]

@mayeut Any opinion on this ? (since it seems to be an area you touched couple years ago)

[rouault]

other candidate fix for same issue in #1616 ...

[mayeut]

If I'm re-reading the description in #1547 correctly, the presence of SOP markers is still optional when (p_tcp->csty & J2K_CP_CSTY_SOP) != 0U, i.e. they are allowed, not required.
I might be missing something but this check implies they're required.

[jeffspahr]

Good catch. I removed the SOP-specific hard failure in 6e67639 so zero-progress empty packets now always terminate packet iteration as header exhaustion instead of making optional SOP absence fatal. I also added unit coverage using an openjpeg-data tolerated EOF fixture with the SOP bit set but no SOP markers, plus kept the issue1472 variants returning promptly.

[mayeut]

Any opinion on this ?

I really can't dig much into this.
I can only give an unchecked opinion & the comment left inline.

Compared to #1616, this PR does not require a counter & a threshold to tune but adds - what seems to be - proper handling of end of stream which does work with other tests images and without any tuning which sounds good.

[jeffspahr]

This PR was re-evaluated with Claude Fable 5 (multi-angle review of both the approach and the CVE coverage), and I pushed three follow-up commits addressing everything it found: a8f8d15, f8430c1, e987f67.

What changed

Stronger, simpler exhaustion guard (a8f8d15): the zero-progress check is now l_header_length == 0 alone. A zero-length empty-packet header can only occur when the packet-header source is exhausted — the codestream remainder in the normal case, or the PPM/PPT buffer in packed-header mode. The previous l_current_data == p_src_data conjunct compared the codestream pointer, which the SOP branch advances before the guard runs; with PPM/PPT + SOP, an exhausted packed-header buffer went undetected while genuine SOP markers remained in the tile body, and the decoder kept fabricating "successful" empty packets. The new predicate covers all header modes uniformly.

The same commit also replaces the single-use opj_packet_progression_t enum with an OPJ_BOOL out-flag matching the neighboring p_is_data_present convention, removes the redundant re-initializations and early returns in opj_t2_decode_packet/opj_t2_skip_packet (the normal fall-through already yields the identical result), hoists the resno_decoded finalization to the single loop-exit site, and moves the flag declaration to the top of its block — the mid-block declaration would have failed GCC builds under the project's -Werror=declaration-after-statement.

Test harness (f8430c1): decodes now go through a temporary file and opj_stream_create_default_file_stream like the other unit tests instead of a hand-rolled in-memory stream; the three malformed Scod variants are table-driven; the fixture Scod patch is anchored at the SOC marker so incidental FF 52 byte pairs in JP2 wrapper boxes cannot be patched by mistake; and the test timeout is raised to 30s so valgrind/sanitizer runs do not flake (it only needs to catch the pre-fix loop, which never returned).

Seed-corpus script (e987f67): an unexpected Scod byte in the upstream seed now warns and skips the derived variants instead of aborting the entire corpus build, and the temp-dir cleanup is a plain mktemp -d + trap.

One reviewer suggestion was tested and deliberately not taken: making opj_t2_finalize_resno_decoded conditional (if (resno_decoded == 0)) like the first_pass_failed fallback. The SOP-bit fixture coverage in this PR caught the problem immediately — a partial decode leaves resno_decoded unequal across components and opj_tcd_mct_decode then rejects the tile, failing dwt_interleave_h.gsr105.jp2. The unconditional assignment is correct: decoded packets never exceed minimum_num_resolutions - 1 (guaranteed by the skip condition in opj_t2_decode_packets), so it never lowers a value, and it is exactly where the pre-fix tolerant iteration converged. The helper now carries a comment documenting that invariant.

Why this is the right fix for CVE-2023-39327

The root cause is that the bio reader fabricates 0-bits at EOF, so opj_t2_read_packet_header() could report successful empty packets indefinitely without consuming any input, and the caller iterated packets against unchanged state. With this PR, every packet iteration either consumes at least one byte from the packet-header source or terminates the tile's packet iteration on the spot — in all header modes (plain, SOP, PPM/PPT) and on both the decode and skip paths. Per-tile work is therefore bounded by the tile's actual data, the outer loops are bounded by SIZ/SOT, and tolerated truncated decodes keep the same resno_decoded end state as before, so no tuning threshold is needed. The Scod 0x01/0x03/0x07 variants are locked in by the unit test and the derived fuzz seeds.

Verification

• Library and test compile warning-free with the project's GCC warning set, including -Werror=declaration-after-statement
• ctest -R "testissue1472_noeph|testempty" passes, including the Marrin.jp2 and dwt_interleave_h.gsr105.jp2 SOP-bit tolerated-EOF fixtures against openjpeg-data
• tests/fuzzers/build_seed_corpus.sh verified on both paths: normal (all three issue1472 seeds land in the zip) and unexpected-Scod (warns, skips the variants, corpus build continues)