[Swift] Harden input validation in FlatBuffers & FlexBuffers runtime … - #9271
Open
krishna28238-arch wants to merge 1 commit into
Open
krishna28238-arch wants to merge 1 commit into
krishna28238-arch wants to merge 1 commit into
Conversation
…readers Fixes several out-of-bounds read paths and correctness issues that are reachable when reading malformed buffers through the non-verifying APIs: FlatBuffers: - Table.compare (two-offset variants) iterated one byte past the shared length of the compared strings and its length-only fallthrough was inverted relative to the loop's (second - first) convention. - ByteBuffer.readString trapped or read out of bounds on negative / out-of-range counts since its bounds check was assert-only (stripped in release); it now returns nil. readSlice and withUnsafePointerToSlice now use preconditions so release builds fail deterministically instead of reading out of bounds. - String(bb, o:) read its length prefix without checking the offset against the buffer; corrupted offsets now resolve to an empty string. - getRoot / getPrefixedSizeRoot failed with an unhelpful unwrap trap or read out of bounds on buffers too small to hold a root offset; they now fail fast with a descriptive message. FlexBuffers: - FlexBuffersWriter stored String.count (grapheme clusters) as the byte length of strings and map keys while writing UTF-8 bytes, truncating non-ASCII strings and breaking sorted-map lookups for non-ASCII keys; lengths are now UTF-8 byte counts. - Value.elementWidth searched byte widths 1, 3, 5, 7 instead of 1, 2, 4, 8, so every offset wider than a byte fell back to w64; the search now uses power-of-two widths matching the C++ implementation. - binarySearch over map keys probed index == count when the target was greater than every key, reading past the keys vector. - getRoot accepted any byte width (0-255) from the trailing root byte, computing a negative root offset for large widths; invalid widths now throw FlexBuffersErrors.invalidByteWidth. - TypedVector.mapKeys propagated a corrupted child byte width (also a numericCast overflow trap); structurally invalid key vectors are now treated as empty. - TypedVector.compare used strcmp against an unchecked raw pointer, reading past the end of the buffer for non-null-terminated keys; the comparison is now bounded by the buffer size. Adds regression tests for each fix under tests/swift/Tests.
|
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. |
Author
|
i have done the cla agreement |
This branch has not been deployed
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.
This PR hardens input validation across the Swift FlatBuffers and FlexBuffers runtime readers. The non-verifying read paths (getRoot, Table, Reference, map/vector readers) perform unchecked pointer arithmetic on data parsed straight out of the buffer, and most bounds checks that do exist are asserts, which are compiled out in release builds. Concretely, on malformed or truncated buffers the following were reachable:
FlatBuffers
FlexBuffers
Well-formed buffers keep their exact current behavior: all existing tests, including the byte-for-byte flexbufferGolden / allTypesGolden comparisons against C++-generated fixtures, pass unchanged. No code generators were touched, so no code regeneration is required.
Testing