Skip to content

opj_tcd: fix out-of-bounds read in opj_tcd_get_decoded_tile_size() when the reduce factor equals numresolutions (#1631)#1640

Open
Alearner12 wants to merge 1 commit into
uclouvain:masterfrom
Alearner12:fix-1631-resolution-factor-oob
Open

opj_tcd: fix out-of-bounds read in opj_tcd_get_decoded_tile_size() when the reduce factor equals numresolutions (#1631)#1640
Alearner12 wants to merge 1 commit into
uclouvain:masterfrom
Alearner12:fix-1631-resolution-factor-oob

Conversation

@Alearner12

Copy link
Copy Markdown

PR: Fix OOB read in opj_tcd_get_decoded_tile_size() when reduce factor == numresolutions

Base: 21b70b0 (v2.5.4-23) · Branch: fix-1631-resolution-factor-oob · Diff: j2k.c +12/-2, tcd.c +7/-1
Closes: #1631 (reported by @5asever40-a11y, crediting OH HAN GUEL and KANG DAEUN)


Bug

opj_tcd_get_decoded_tile_size() (tcd.c:1421) indexes
l_tile_comp->resolutions + l_tile_comp->minimum_num_resolutions - 1.
When minimum_num_resolutions == 0 this is resolutions[-1], an out-of-bounds
read dereferenced at tcd.c:1426 the SEGV reported in #1631 (ASan, READ on an
unmapped address).

minimum_num_resolutions is computed in opj_tcd_init_tile() (tcd.c:880):

if (l_tccp->numresolutions < l_cp->m_specific_param.m_dec.m_reduce) {
    l_tilec->minimum_num_resolutions = 1;
} else {
    l_tilec->minimum_num_resolutions =
        l_tccp->numresolutions - l_cp->m_specific_param.m_dec.m_reduce;   // 0 when equal
}

The intended invariant is m_reduce < numresolutions — COD parsing enforces it
in opj_j2k_read_SPCod_SPCoc() (j2k.c:10979) by rejecting
m_reduce >= numresolutions. But this guard uses <, so when
m_reduce == numresolutions it falls into the else branch and yields 0.

m_reduce reaches that equal-to value through
opj_j2k_set_decoded_resolution_factor() (j2k.c:12556), which writes
m_reduce = res_factor before validating and then returns OPJ_FALSE:

p_j2k->m_cp.m_specific_param.m_dec.m_reduce = res_factor;   /* mutated first */
...
    if (res_factor >= max_res) { ...; return OPJ_FALSE; }    /* rejected, but state already changed */

So a rejected set_decoded_resolution_factor() leaves the decoder in a
corrupted state, and a later opj_read_tile_header() trips the OOB.

Reachability (honest scoping)

This is not reachable through opj_decompress or the in-tree OSS-Fuzz
harnesses (opj_decompress_fuzzer_{J2K,JP2}): those take the full-decode path,
where COD parsing rejects m_reduce >= numresolutions up front
(opj_decompress -r 3 on a 3-resolution image exits cleanly:
"The number of resolutions to remove (3) is greater or equal than the number of
resolutions of this component (3)"
). It is reachable from application code that
calls opj_set_decoded_resolution_factor() with an out-of-range factor and then
proceeds to opj_read_tile_header() without checking the OPJ_FALSE return.
Severity is correspondingly low the standard decode path is unaffected — but a
public setter should not corrupt decoder state on its rejection path, and the
tcd.c guard is internally inconsistent with the COD-parse invariant.

Fix

Two small, independent changes (either alone prevents the crash; together they
fix the root cause and harden the point of use):

  1. opj_j2k_set_decoded_resolution_factor() — root cause. Validate every
    component before mutating any state, so a rejected factor is a no-op and
    leaves m_reduce and the per-component factor values unchanged. (This also
    closes a pre-existing partial-state issue: comps[i].factor was being set
    incrementally before a later component could fail validation.)

  2. opj_tcd_init_tile() — defense in depth. Use <= so
    minimum_num_resolutions is clamped to >= 1 whenever
    m_reduce >= numresolutions, matching the >= invariant the COD parser
    already enforces. This guarantees opj_tcd_get_decoded_tile_size() can never
    read resolutions[-1], regardless of how m_reduce was set.

Verification

Built with -fsanitize=address at 21b70b0. A harness reproducing the #1631
sequence (read_headerset_decoded_resolution_factor(rf)
read_tile_header) on the reported crash input:

reduce factor before after
1, 2 (valid) ok ok — identical tile sizes (13924 / 3600)
3 (== numresolutions) SEGV @ tcd.c:1426 no crash, clean read
4 (> numresolutions) ok (already clamped) ok
  • Each fix independently stops the SEGV. With only the tcd.c clamp (setter
    left buggy, so m_reduce is still corrupted to 3), rf=3 no longer crashes
    it clamps to minimum_num_resolutions = 1. With the setter fix, m_reduce is
    never corrupted in the first place.
  • No regression on the normal path: opj_decompress -r 0 / -r 2 decode the
    file unchanged; opj_decompress -r 3 still exits cleanly via the COD-parse
    check. Valid reduce factors (m_reduce < numresolutions) are byte-for-byte
    unaffected — the tcd.c change only diverges from current behavior in the
    m_reduce == numresolutions case, which valid decoding never reaches.

…en the reduce factor equals numresolutions (uclouvain#1631)

This patch addresses an OOB read triggered when minimum_num_resolutions
underflows to 0. It fixes the setter opj_j2k_set_decoded_resolution_factor()
to validate against all components before mutating decoder state, and updates
the boundary check in opj_tcd_init_tile() to use <= to ensure clamping.
@5asever40-a11y

5asever40-a11y commented Jun 1, 2026 via email

Copy link
Copy Markdown

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.

2 participants