Fix integer overflow in STL parser sanity check#4790
Open
jimmy-ly00 wants to merge 1 commit into
Open
Conversation
The sanity check in LoadMeshFromSTL computes expectedBinaryFileSize as:
int expectedBinaryFileSize = numTriangles * 50 + 84;
Both numTriangles and 50 are int, so the multiplication is performed in
32-bit signed arithmetic and can overflow. For example, numTriangles =
85899346 yields 85899346 * 50 = 4294967300, which wraps to 4 in 32-bit.
Adding 84 gives 88, which can match a crafted 88-byte file and bypass
the check. The parser then tries to read ~4 GB of triangles from the
88-byte heap buffer.
Fix: cast numTriangles to long long before the multiplication so the
arithmetic is performed in 64-bit, preventing the overflow. Valid STL
files are unaffected because the result is identical when it fits in
32 bits.
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.
The sanity check in LoadMeshFromSTL computes expectedBinaryFileSize as:
Both numTriangles and 50 are int, so the multiplication is performed in 32-bit signed arithmetic and can overflow. For example, numTriangles = 85899346 yields 85899346 * 50 = 4294967300, which wraps to 4 in 32-bit. Adding 84 gives 88, which can match a crafted 88-byte file and bypass the check. The parser then tries to read ~4 GB of triangles from the 88-byte heap buffer.
Fix: cast numTriangles to long long before the multiplication so the arithmetic is performed in 64-bit, preventing the overflow. Valid STL files are unaffected because the result is identical when it fits in 32 bits.