Repository navigation
Fix missing bounds checks in Table.VectorLen/Vector (Python, Go, Java) - #9411
behnam1990adel-hash wants to merge 1 commit into
Conversation
|
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). View this failed invocation of the CLA check for more information. For the most up to date status, view the checks section at the bottom of the pull request. |
There was a problem hiding this comment.
Code Review
This pull request introduces bounds checking to vector length and vector data retrieval functions across Go, Java, and Python implementations of FlatBuffers to prevent crashes and out-of-bounds reads from corrupted or malicious buffers, along with corresponding regression tests. However, the feedback highlights critical security vulnerabilities in the new checks across all three languages, where additions like off += t.Pos or negative offsets can cause integer overflows or bypasses, allowing malicious buffers to still evade the bounds checks. The reviewer has provided robust, overflow-safe suggestions to resolve these issues.
6de3a14 to
92a6986
Compare
92a6986 to
3fbda24
Compare
|
Good catch — confirmed the bypass and fixed it in Go and Java with the overflow-safe pattern you suggested (compare with subtraction before any addition). Python doesn't have this exploitable gap since it has no fixed-width wraparound, but I applied the same pattern there too for consistency. Added a dedicated regression test for the wraparound case in all three languages. Pushed. |
Problem
Table.VectorLen()andTable.Vector()in the Python, Go, and Java implementations read a vector's relative-offset and length fields straight out of the buffer with no bounds checking. If either field is corrupted, nothing stops a crash.Go panics (
slice bounds out of range), Java throwsIndexOutOfBoundsException, Python raisesstruct.error. None of the three languages ship a verifier, so generated code has no way to catch this before it happens.ByteVector()ingo/table.goalready got this treatment in #8776.VectorLen()andVector()are called for every single vector field access and were still unguarded.Fix
Same pattern as the existing
ByteVectorfix: validate that both the relative-offset read and the length read land inside the buffer before trusting either value.VectorLen()returns0instead of an attacker-controlled length.Vector()returns a position past the end of the buffer instead of a wild address when it can't safely compute the real one.Same logic, same two-stage check, in all three languages.
Before / after
With the vector's length field corrupted to
0x7FFFFFFF:InventoryLength()returns2147483647with no error, thenInventory(0)throwsstruct.error: unpack_from requires a buffer of at least 37 bytes...InventoryLength()returns0Corrupting the relative-offset field instead reproduces the same class of bug immediately on the call itself: Go panics inside
VectorLen()(slice bounds out of range), Java throwsIndexOutOfBoundsException, Python raisesstruct.error.Standalone repro
No dependencies beyond the Python standard library, does not require the
flatbufferspackage to be installed:Testing
tests/py_test.py,tests/go_test.go,java/src/test/java/JavaTest.javacovering both the corrupted-length and corrupted-offset cases.Related work
A University of Hamburg master's thesis ("Security Evaluation and Vulnerability Assessment of FlatBuffers", Finn Sell) independently documented this exact crash behavior for Go, Java, and Python as part of a broader language comparison. This PR doesn't claim to be the first to notice the gap — it's a concrete fix for it, following the same approach already merged for
ByteVector.