fix: validate face vertex indices in sequential decoder to prevent OOB access#1167
Open
mohammadmseet-hue wants to merge 1 commit intogoogle:mainfrom
Open
fix: validate face vertex indices in sequential decoder to prevent OOB access#1167mohammadmseet-hue wants to merge 1 commit intogoogle:mainfrom
mohammadmseet-hue wants to merge 1 commit intogoogle:mainfrom
Conversation
…coder The sequential mesh decoder reads face vertex indices from the bitstream without validating that they fall within [0, num_points). When a crafted Draco file contains face indices exceeding num_points, subsequent attribute access via these indices causes out-of-bounds heap reads (and potentially writes during attribute deduplication or other operations). ASan confirms the OOB: ERROR: AddressSanitizer: heap-buffer-overflow READ of size 12 at <addr> <addr> is located 1128 bytes after 72-byte region This affects all four index decoding paths (uint8, uint16, uint32 varint, uint32 direct) and the compressed index path (DecodeAndDecompressIndices). Fix: add a validation pass after decoding connectivity that checks all face vertex indices are within the valid range [0, num_points).
Author
ASan ConfirmationBuilt with The 72-byte attribute buffer (6 points × 12 bytes/entry) was accessed at index 100 (offset 1200) — 1128 bytes past the buffer end. The face vertex index |
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
The sequential mesh decoder reads face vertex indices from the Draco bitstream without validating that they fall within
[0, num_points). When a crafted Draco file contains out-of-range indices, attribute access viaGeometryAttribute::GetValue()causes an out-of-bounds heap read.ASan Proof
A 72-byte attribute buffer (6 points × 12 bytes) was accessed at index 100 (offset 1200) — 1128 bytes past the end.
Root Cause
mesh_sequential_decoder.cclines 74-125: all four index decoding paths (uint8,uint16,uint32varint,uint32direct) store indices from the stream intoface[j]without checkingval < num_points. The compressed index path (DecodeAndDecompressIndices) has the same issue.Impact
Any application that decodes attacker-provided
.drcfiles is affected. The OOB read can leak heap data. Combined with attribute write operations (deduplication, transforms), this could potentially become an OOB write.Fix
Added a validation pass after decoding connectivity that checks all face vertex indices are within
[0, num_points).Separate from PR #1166
This is a different root cause from the integer overflow in PR #1166:
num_faces * 3overflow → wrong allocation size → heap overflow (WRITE)