fix(edgebreaker): validate num_encoded_symbols against remaining bit budget (fixes #1169) - #1223
Open
jdymitarai wants to merge 1 commit into
Open
Conversation
…budget (fixes google#1169) In MeshEdgebreakerDecoderImpl::DecodeConnectivity(), num_encoded_symbols is read from untrusted input. Prior to this commit, num_faces was bounded only by num_encoded_symbols, but num_encoded_symbols itself was never bounded against the remaining bytes in the bitstream. A malformed input declaring an astronomical num_encoded_symbols could trigger massive memory allocation in processed_corner_ids_.reserve(num_faces), processed_connectivity_corners_.reserve(num_faces), and corner_table_->Reset(), consuming gigabytes of memory and causing Denial of Service (CWE-789) with a tiny (<100 byte) input file. Since each edgebreaker symbol requires at least 1 bit in the remaining bitstream, this commit validates that num_encoded_symbols does not exceed the remaining bit budget (remaining_size() * 8). Also adds a unit test RejectExcessiveSymbols with the reproducer payload to verify that corrupt inputs are cleanly rejected. Fixes google#1169.
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
1 participant
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 of Changes
Fixes #1169 (CWE-789: Uncontrolled Memory Allocation in
MeshEdgebreakerDecoderImpl::DecodeConnectivity()allowing DoS with tiny input).Problem
MeshEdgebreakerDecoderImpl::DecodeConnectivity()readsnum_encoded_symbolsandnum_facesfrom untrusted input and proceeds to allocate substantial internal structures:processed_corner_ids_.reserve(num_faces);processed_connectivity_corners_.reserve(num_faces);corner_table_->Reset(num_faces, ...);While
num_facesis checked againstmax_encoded_faces(num_encoded_symbols * 4 / 3),num_encoded_symbolswas never validated against the remaining size of the input buffer. As demonstrated in #1169, a crafted 61-byte input claiming an astronomical symbol count triggers multi-gigabyte allocations and immediate out-of-memory crashes / DoS in applications and WebAssembly decoders.Fix
Because each Edgebreaker symbol requires at least 1 bit in the remaining bitstream (e.g.
TOPOLOGY_Cis 1 bit, others 3 bits, or at least 1 bit under prediction/entropy decoding), validate thatnum_encoded_symbolsdoes not exceedremaining_size() * 8. If it does,DecodeConnectivity()immediately returnsfalsebefore any memory allocation occurs.Test
Added
MeshEdgebreakerEncodingTest.RejectExcessiveSymbolswith the reproducer payload from #1169 to ensure malformed buffers with excessive symbol counts are safely rejected.