Repository navigation
[FIX] Widen avc cc_count to uint16_t/u16 to prevent overflow on multi-SEI frames - #2380
Open
muntasir-396 wants to merge 1 commit into
Open
muntasir-396 wants to merge 1 commit into
muntasir-396 wants to merge 1 commit into
Conversation
muntasir-396
force-pushed
the
fix-avc-cc-count-wrap
branch
from
October 16, 2026 16:16
043a1ba to
c347428
Compare
…-SEI frames (CCExtractor#2379) In commit c1043ca (CCExtractor#2359), MAX_CC_COUNT and HDCC_MAX_TRIPLETS_PER_SLOT were raised from 31 to 310 to allow H.264 access units with multiple SEI messages to buffer their caption triplets. However, avc_ctx->cc_count was stored as an unsigned char in C and u8 in Rust. When an access unit accumulates multiple SEIs crossing 255 triplets, ctx->cc_count wrapped around in C (causing buffer corruption and lost triplets) and panicked in debug mode in Rust. - src/lib_ccx/avc_functions.h: widen avc_ctx.cc_count from unsigned char to uint16_t. - src/lib_ccx/avc_functions.c: guard ctx->cc_count addition against overflow in copy_ccdata_to_buffer(). - src/rust/src/avc/common_types.rs: widen AvcContextRust.cc_count from u8 to u16. - src/rust/src/avc/core.rs: update copy_ccdata_to_buffer() with bounded u16 arithmetic and add unit test.
muntasir-396
force-pushed
the
fix-avc-cc-count-wrap
branch
from
October 16, 2026 17:17
c347428 to
b5fe1ae
Compare
Collaborator
CCExtractor CI platform finished running the test files on linux. 171/237 tests matched the approved output:
66 tests do not match the approved output. That is the pass/fail verdict. Whether this branch caused it is a separate question, answered below.
Compared with the tip of master — test 9642, commit 97a0d24:
Compared with the commit this branch was cut from: the same run as the tip of master (test 9642), so the comparison above already covers it. No test changes behaviour relative to the tip of master: every failure above fails there too, byte for byte. The approved output for those tests is out of date, which is a baseline to review rather than a regression in this branch. |
Collaborator
CCExtractor CI platform finished running the test files on windows. 171/237 tests matched the approved output:
66 tests do not match the approved output. That is the pass/fail verdict. Whether this branch caused it is a separate question, answered below.
Compared with the tip of master — test 9643, commit 97a0d24:
Compared with the commit this branch was cut from: the same run as the tip of master (test 9643), so the comparison above already covers it. No test changes behaviour relative to the tip of master: every failure above fails there too, byte for byte. The approved output for those tests is out of date, which is a baseline to review rather than a regression in this branch. |
Author
CI Status Note
Ready for maintainer review whenever convenient! |
Author
|
Fixes #2379 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
In raising this pull request, I confirm the following (please check boxes):
Reason for this PR:
Sanity check:
Summary
In commit
c1043ca7(#2359),MAX_CC_COUNTandHDCC_MAX_TRIPLETS_PER_SLOTwere increased from 31 to 310 to allow H.264 access units with multiple SEI messages to buffer their caption triplets.However,
avc_ctx->cc_countremained typed asunsigned charin C (avc_functions.h:6) andu8in Rust (common_types.rs:7). Because an 8-bit unsigned integer caps at 255:ctx->cc_count += new_cc_countwraps modulo 256 (260 % 256 = 4), corrupting the buffer index, overwriting the start of the buffer, and silently losing 256 triplets.core.rs:445),ctx.cc_count += new_cc_count as u8panics in debug builds due to overflow and wraps in release mode.Changes
src/lib_ccx/avc_functions.h: widencc_countinstruct avc_ctxfromunsigned chartouint16_t.src/rust/src/avc/common_types.rs: widencc_countinAvcContextRustfromu8tou16.src/rust/src/avc/core.rs: updatecopy_ccdata_to_buffer()with safeu16addition and add unit testtest_copy_ccdata_to_buffer_beyond_u8_max().