core: fix SEGV in param_qcd due to underflow when num_subbands is 0#267
core: fix SEGV in param_qcd due to underflow when num_subbands is 0#267bwendling wants to merge 1 commit intoaous72:masterfrom
Conversation
In `ojph::local::param_qcd::get_Kmax` and `ojph::local::param_qcd::get_irrev_delta`, a segmentation fault occurred when processing a QCD/QCC marker segment with zero subbands. When `num_subbands` is 0, the condition `idx >= num_subbands` evaluates to true. The code then attempted to clamp the index using `idx = num_subbands - 1`. Since `num_subbands` is an unsigned 32-bit integer, this resulted in an integer underflow to `0xFFFFFFFF`. Subsequent access to `SPqcd.u8[idx]` caused an out-of-bounds read and a crash. This patch adds an explicit check for `num_subbands == 0` within the bounds-checking logic. If no subbands are present, it now triggers an `OJPH_ERROR` to safely abort the operation, as a QCD/QCC segment without quantization step sizes is invalid. Verified that this fix prevents the ASAN SEGV and passes the project's test suite. Fixes: https://issues.oss-fuzz.com/issues/477315155 Co-authored-by: CodeMender <codemender-patching@google.com> Signed-off-by: Bill Wendling <morbo@google.com>
|
Hi Bill, Thank you for putting this PR. I wonder why this was not captured earlier when the QCD/QCC marker segments are read; there is a check in This is worth more investigation. Perhaps, we take it offline. Cheers, |
|
Hi Aous, We (the CodeMender people) actually encountered this while performing fuzzy testing on the openexr project. It looks like they're using OpenJPH v0.26.3. The check you showed seems to be in newer code. This maybe d479df2? If this PR is already covered by that check, then feel free to close this and I apologize for the spammage. |
|
Thank you for the feedback. |
|
Please reopen this if the problem persist. Thank you. |
2 participants
In
ojph::local::param_qcd::get_Kmaxandojph::local::param_qcd::get_irrev_delta, a segmentation fault occurred when processing a QCD/QCC marker segment with zero subbands.When
num_subbandsis 0, the conditionidx >= num_subbandsevaluates to true. The code then attempted to clamp the index usingidx = num_subbands - 1. Sincenum_subbandsis an unsigned 32-bit integer, this resulted in an integer underflow to0xFFFFFFFF. Subsequent access toSPqcd.u8[idx]caused an out-of-bounds read and a crash.This patch adds an explicit check for
num_subbands == 0within the bounds-checking logic. If no subbands are present, it now triggers anOJPH_ERRORto safely abort the operation, as a QCD/QCC segment without quantization step sizes is invalid.Verified that this fix prevents the ASAN SEGV and passes the project's test suite.
Fixes: https://issues.oss-fuzz.com/issues/477315155
Co-authored-by: CodeMender codemender-patching@google.com
Signed-off-by: Bill Wendling morbo@google.com