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

re-order shift/compare in FastHuf to prevent undefined shift overflow #819

Merged

Conversation

peterhillman
Copy link
Contributor

UndefinedBehaviorSanitizer warns at bufferBack = bufferBack << numBits when numBits is too large, as that is an undefined operation. In practice this is safe because bufferBack was guaranteed to be set to zero in a subsequent check.
This change reorders operations to prevent the shift being computed in this case, which prevents the warning.

Signed-off-by: Peter Hillman peterh@wetafx.co.nz

… warning

Signed-off-by: Peter Hillman <peterh@wetafx.co.nz>
Copy link
Member

@cary-ilm cary-ilm left a comment

Choose a reason for hiding this comment

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

LGTM.

@cary-ilm
Copy link
Member

If this was inspired by an ozz-fuzz issue, can you add a reference to it in the comments here, for tracking purposes?

@peterhillman
Copy link
Contributor Author

If this was inspired by an ozz-fuzz issue, can you add a reference to it in the comments here, for tracking purposes?
There's no oss-fuzz issue here. This could be reproduced with IlmImfTest testHuf with the sanitizer running

@cary-ilm cary-ilm merged commit 5c74ae8 into AcademySoftwareFoundation:master Aug 18, 2020
@peterhillman peterhillman deleted the fasthuf_reorder branch September 30, 2020 04:33
peterhillman added a commit to peterhillman/openexr that referenced this pull request Dec 7, 2020
… warning (AcademySoftwareFoundation#819)

Signed-off-by: Peter Hillman <peterh@wetafx.co.nz>

Co-authored-by: Cary Phillips <cary@ilm.com>
Signed-off-by: Peter Hillman <peterh@wetafx.co.nz>
cary-ilm added a commit that referenced this pull request Dec 30, 2020
…tests (#875)

* ignore unused bits in B44 mode detection

Signed-off-by: Peter Hillman <peterh@wetafx.co.nz>

* apply #832: use unsigned values in shift to prevent undefined behavior

Signed-off-by: Peter Hillman <peterh@wetafx.co.nz>

* apply #818: compute Huf codelengths using 64 bit to prevent shift overflow

Signed-off-by: Peter Hillman <peterh@wetafx.co.nz>

* apply #817: double-check unpackedBuffer created in DWA uncompress

Signed-off-by: Peter Hillman <peterh@wetafx.co.nz>

* apply #820: suppress sanitizer warnings when writing invalid enums

Signed-off-by: Peter Hillman <peterh@wetafx.co.nz>

* apply #825: Avoid overflow in calculateNumTiles when size=MAX_INT

Signed-off-by: Peter Hillman <peterh@wetafx.co.nz>

* apply #826: restrict maximum tile size to INT_MAX byte limit

Signed-off-by: Peter Hillman <peterh@wetafx.co.nz>

* apply #827: lighter weight reading of Luma-only images via RgbaInputFile

Signed-off-by: Peter Hillman <peterh@wetafx.co.nz>

* refactor channel filling in InputFile API with tiled source

Signed-off-by: Peter Hillman <peterh@wetafx.co.nz>

* handle edge-case of empty framebuffer

Signed-off-by: Peter Hillman <peterh@wetafx.co.nz>

* apply #829: fix buffer overflow check in PIZ decompression

Signed-off-by: Peter Hillman <peterh@wetafx.co.nz>

* Use Int64 in dataWindowForTile to prevent integer overflow (#831)

* Use Int64 in dataWindowForTile to prevent integer overflow

Signed-off-by: Peter Hillman <peterh@wetafx.co.nz>

* use signed 64 bit instead for dataWindow calculation

Signed-off-by: Peter Hillman <peterh@wetafx.co.nz>

Co-authored-by: Cary Phillips <cary@ilm.com>
Signed-off-by: Peter Hillman <peterh@wetafx.co.nz>

* prevent overflow in hufUncompress if nBits is large (#836)

Signed-off-by: Peter Hillman <peterh@wetafx.co.nz>

* add sanity check for reading multipart files with no parts (#840)

Signed-off-by: Peter Hillman <peterh@wetafx.co.nz>

* reduce B44 _tmpBufferSize (was allocating two bytes per byte) (#843)

Signed-off-by: Peter Hillman <peterh@wetafx.co.nz>

* check for valid Huf code lengths (#849)

* check for valid Huf code lengths
* test non-fast huf decoder in testHuf

Signed-off-by: Peter Hillman <peterh@wetafx.co.nz>

* check 1 part files with 'nonimage' bit have type attribute (#860)

Signed-off-by: Peter Hillman <peterh@wetafx.co.nz>

Co-authored-by: Cary Phillips <cary@ilm.com>
Signed-off-by: Peter Hillman <peterh@wetafx.co.nz>

* Fix overflow computing deeptile sample table size (#861)


Signed-off-by: Peter Hillman <peterh@wetafx.co.nz>

* re-order shift/compare in FastHuf to prevent undefined shift overflow warning (#819)

Signed-off-by: Peter Hillman <peterh@wetafx.co.nz>

Co-authored-by: Cary Phillips <cary@ilm.com>
Signed-off-by: Peter Hillman <peterh@wetafx.co.nz>

* more elegant exception handling in exrmaketiled (#841)

Signed-off-by: Peter Hillman <peterh@wetafx.co.nz>

* check EXRAllocAligned succeeded to allocate ScanlineInputFile lineBuffers (#844)

Signed-off-by: Peter Hillman <peterh@wetafx.co.nz>

* test channels are DCT compressed before DWA decompression (#845)

Signed-off-by: Peter Hillman <peterh@wetafx.co.nz>

* Merge ABI-compatible changes from #842

Signed-off-by: Peter Hillman <peterh@wetafx.co.nz>

Co-authored-by: Cary Phillips <cary@ilm.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.

None yet

2 participants