fix(rla): lots of additional validity checking and safety#5094
Merged
lgritz merged 2 commits intoAcademySoftwareFoundation:mainfrom Mar 23, 2026
Merged
fix(rla): lots of additional validity checking and safety#5094lgritz merged 2 commits intoAcademySoftwareFoundation:mainfrom
lgritz merged 2 commits 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>
Collaborator
Author
|
Comments or objections? |
jessey-git
approved these changes
Mar 23, 2026
Contributor
jessey-git
left a comment
jessey-git
left a comment
There was a problem hiding this comment.
These changes look fine minus 1 nit.
Signed-off-by: Larry Gritz <lg@larrygritz.com>
lgritz
added a commit
to lgritz/OpenImageIO
that referenced
this pull request
Mar 23, 2026
…twareFoundation#5094) * 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. --------- Signed-off-by: Larry Gritz <lg@larrygritz.com>
lgritz
added a commit
to lgritz/OpenImageIO
that referenced
this pull request
Mar 24, 2026
…twareFoundation#5094) * 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. --------- 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
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.
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.