fix uberz offset to pointer out of bounds write#9853
Closed
Sakura-501 wants to merge 1 commit intogoogle:mainfrom
Closed
fix uberz offset to pointer out of bounds write#9853Sakura-501 wants to merge 1 commit intogoogle:mainfrom
Sakura-501 wants to merge 1 commit intogoogle:mainfrom
Conversation
Make ReadableArchive pointer conversion size-aware so release builds validate the decompressed archive before rewriting any offsets into pointers. The previous implementation trusted attacker-controlled specsOffset, flagsOffset, packageOffset, and nameOffset fields and rewrote them without bounds checks, which allowed malformed uberz archives to drive out-of-bounds reads and writes. Add regression tests for top-level specs offsets and nested flag name offsets, and update gltfio plus the uberz tool to pass the decompressed archive size into the validator.
pixelflinger
added a commit
that referenced
this pull request
Apr 14, 2026
- Implement fixes inspired by PR #9853 to make convertOffsetsToPointers size-aware and prevent OOB reads and writes. - Change function signature to return bool instead of void, allowing graceful error propagation instead of runtime aborts. - Replace FILAMENT_CHECK_PRECONDITION with explicit checks that log errors and return false. - Update ArchiveCache in gltfio and main in tools/uberz to handle failure. - Add unit tests to verify rejection of invalid offsets.
Collaborator
|
The fix was incomplete. See #9896 for a more complete fix. |
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.
Summary
This PR fixes out-of-bounds reads and writes in
uberz::convertOffsetsToPointersby making archive pointer conversion size-aware.Vulnerability
convertOffsetsToPointersrewrote attacker-controlled offsets from a decompressed uberz archive into live pointers without validating that the offsets remained inside the archive buffer. In release builds the existingassert_invariantchecks compile away, so malformed values such asspecsOffset,flagsOffset,packageOffset, ornameOffsetcould force the converter to write invalid pointers into the archive structure and then continue traversing those invalid addresses.Real call sites include:
gltfio::ArchiveCache::loadtools/uberzAny application or service that loads untrusted
.uberzcontent could therefore crash or hit broader memory corruption.Fix
The patch changes the conversion API to accept the decompressed archive size and validates all offset-derived regions before any pointer rewriting happens:
specsarray range before convertingspecsOffset;flagsarray, package range, and NUL-terminated flag name string;gltfioandtools/uberzto pass the decompressed archive size into the validator.Tests
Added
libs/uberz/tests/test_ReadableArchive.cppwith regression coverage for:specsOffsetthat points outside the archive buffer;flag.nameOffsetthat points outside the archive buffer.Why this PR is scoped separately
This change only addresses uberz archive validation and call-site propagation. It is intentionally separate from the two independent KTX deserialization fixes.