fix(rla): lots of additional validity checking and safety#5094
Open
lgritz wants to merge 1 commit intoAcademySoftwareFoundation:mainfrom
Open
fix(rla): lots of additional validity checking and safety#5094lgritz wants to merge 1 commit intoAcademySoftwareFoundation:mainfrom
lgritz wants to merge 1 commit intoAcademySoftwareFoundation:mainfrom
Conversation
* Validity-check resolution of RLA files with check_open. RLA file headers contain int16_t values for left & right (and top/bottom) window coordinate, leading to a maximum resolution of 2^16-1. * Fix potential bug with sign extension in RLE decoding -- if a signed char is -128, negating it can't make signed char 128 (no such thing), so must widen the var to an int. * Fix potential bug by detecting when the number of matte or auxiliary bits is 0, but the number of matte or aux channels, respectively, is not. * Better bounds checking in decode_channel_group. We did the checks before, but after some accesses that would have been out of bounds! Move the checks earlier than all the accesses. It actually looks like was the result of a cut and paste error long ago. * More care in read_native_scanline for checking valid scanline numbers, offset into m_sot, and check whether ioseek succeeded (i.e. whether the offsets loaded from the file are within the range of the size of the file). Code and fixes all are from my own brain, but some of the spots with bounds issues were identified in part by conversation with Claude Code Opus 4.6. Assisted-by: Claude Code / Opus 4.6 Signed-off-by: Larry Gritz <lg@larrygritz.com>
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
1 participant
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.
Validity-check resolution of RLA files with check_open. RLA file headers contain int16_t values for left & right (and top/bottom) window coordinate, leading to a maximum resolution of 2^16-1.
Fix potential bug with sign extension in RLE decoding -- if a signed char is -128, negating it can't make signed char 128 (no such thing), so must widen the var to an int.
Fix potential bug by detecting when the number of matte or auxiliary bits is 0, but the number of matte or aux channels, respectively, is not.
Better bounds checking in decode_channel_group. We did the checks before, but after some accesses that would have been out of bounds! Move the checks earlier than all the accesses. It actually looks like was the result of a cut and paste error long ago.
More care in read_native_scanline for checking valid scanline numbers, offset into m_sot, and check whether ioseek succeeded (i.e. whether the offsets loaded from the file are within the range of the size of the file).
Code and fixes all are from my own brain, but some of the analysis of which spots have bounds issues were identified in part by conversation with Claude Code Opus 4.6.