Replace boost::iterator_facade with explicit iterator definition for pcp/iterator.h#2328
Merged
pixar-oss merged 1 commit intoPixarAnimationStudios:devfrom Jul 27, 2023
Merged
Conversation
boost::iterator_facade with explicit iterator definitionboost::iterator_facade with explicit iterator definition for pcp/iterator.h
Contributor
|
Filed as internal issue #USD-8089 |
2 tasks
fa18738 to
9856497
Compare
musicinmybrain
pushed a commit
to musicinmybrain/OpenUSD
that referenced
this pull request
Apr 7, 2026
…Studios#2328) Three classes of signed integer overflow in the PIZ codec path, all reachable from corrupt `dataWindow` dimensions in the EXR file header. **`wav_2D_encode` / `wav_2D_decode` — wavelet loop pointer arithmetic** `oy` is passed as `int` (value `wcount * nx`, at most ~INT32_MAX after the guard below). Inside the hierarchical wavelet loop the expressions ey = in + oy * (ny - p2) // pointer end-of-row sentinel oy1 = oy * p // row stride at level p oy2 = oy * p2 // row stride at level p2 multiply two values that can each approach INT32_MAX, producing a signed 32-bit product that wraps to a small or negative value. The wrapped value is used as a pointer offset, causing reads and writes through `px` / `py` to land outside the allocated wavelet buffer. Fix: widen by introducing `int64_t oy64 = oy` and using it for all three expressions; `oy1` and `oy2` are also declared `int64_t`. **`wavbuf += nx * ny * wcount` — per-channel buffer advance** `nx`, `ny`, and `wcount` are all `int`. Their triple product overflows int32 for moderately large images, causing subsequent channels to be processed at an incorrect (too-small) offset into the wavelet buffer, corrupting both encode and decode output. Fix: cast to `(uint64_t)` before multiplying. **`wcount * nx` — call-site argument overflow** The fifth argument to `wav_2D_encode` / `wav_2D_decode` is `wcount * nx` (`oy` = y-stride = elements per row). `wcount` is 1 or 2 (`bytes_per_element / 2`); for `wcount = 2` the product overflows int32 when `nx > INT32_MAX / 2`. Fix: add an early bounds check `if (wcount > 0 && nx > INT_MAX / wcount)` that rejects such input as `EXR_ERR_CORRUPT_CHUNK` before any arithmetic is performed. This also keeps `wcount * nx` within int32 range at the call site, ensuring `oy` arrives in the wavelet functions with a valid non-overflowed value. Made-with: Cursor Signed-off-by: Cary Phillips <cary@ilm.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
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.
Description of Change(s)
iterator_facadewith explicit implementation forPcpNodeIterator,PcpPrimIterator, andPcpPropertyIterator._PtrProxytype forPcpNodeIteratorandPcpPrimIterator(whose reference type is a proxy value and not a true reference) to provide a safeoperator->implementation.nullptrinstead of0to default initialize pointer types (https://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#Res-nullptr)boost::reverse_iteratorin some parallel work (Provide adapter forstd::reverse_iteratorto replaceboost::reverse_iteratorfor proxy reference iterators #2333), it was observed that the current implementation ofPcpNodeIteratorhangs whenstd::prevwas used. The source of the hang was not determined but not observed in this implementation.testPcpIteratorhas been augmented to include tests to validate that the increment / decrement operators andstd::prev/std::nextcan be used and produce symmetrical end states.Fixes Issue(s)
boost::iterator_facadeusage with fully specified iterator classes #2305