Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix range check in dwa compressor #1472

Merged
merged 1 commit into from
Jul 10, 2023

Conversation

meshula
Copy link
Contributor

@meshula meshula commented Jun 30, 2023

out-dwaa.zip

I've attached out-dwaa.exr ~ note that I simply renamed it to .zip because github doesn't allow uploading of exr files into an issue. So just rename it back to exr.

anyway :)

The attached file fails to load in OpenEXRCore, because it's got some buffers that exactly line up at their ends, which causes the sanity checks to fail. Changing the comparisons to < instead of <= allows the file to load, and I believe preserves the intent of the checks.

Note that I pre-emptively switched all of the checks to < even though changing only the first one is sufficient to load this particular test file. I'm wondering if someone could sanity check all the checks?

Signed-off-by: Nick Porcino <meshula@hotmail.com>
Copy link
Contributor

@kdt3rd kdt3rd left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, these were added in haste after discovering that the individual size checks were not failing, but the summation of all the offsets caused a past the end access. I do think the logic here can be simplified over time, but was concerned with breaking compatibility, which I then proceeded to do :)

Your fix is fine here, thanks for the suggestion

@kdt3rd kdt3rd merged commit 811c941 into AcademySoftwareFoundation:main Jul 10, 2023
cary-ilm pushed a commit to cary-ilm/openexr that referenced this pull request Jul 25, 2023
Signed-off-by: Nick Porcino <meshula@hotmail.com>
cary-ilm pushed a commit that referenced this pull request Jul 31, 2023
Signed-off-by: Nick Porcino <meshula@hotmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants