Skip to content

[Swift] Harden input validation in FlatBuffers & FlexBuffers runtime … - #9271

Open
krishna28238-arch wants to merge 1 commit into
google:masterfrom
krishna28238-arch:fix/swift-runtime-input-validation
Open

krishna28238-arch wants to merge 1 commit into
google:masterfrom
krishna28238-arch:fix/swift-runtime-input-validation

Conversation

@krishna28238-arch

@krishna28238-arch krishna28238-arch commented Sep 22, 2026

Copy link
Copy Markdown

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

Table.compare off-by-one — both two-offset variants iterated 0...minValue instead of 0..<minValue, reading one byte past the shared length of the compared strings. The length-only fallthrough was also inverted relative to the loop's (second - first) return convention (it returned len1 - len2); it now returns len2 - len1, consistent with the ordering the generated sortVectorOf… code relies on.
ByteBuffer.readString — assert-only bounds check, so release builds read out of bounds (or built a negative-count UnsafeBufferPointer) for corrupted string lengths. Now returns nil for negative / out-of-range index or count (the API already returned optional).
ByteBuffer.readSlice / withUnsafePointerToSlice — assert-only bounds checks are now preconditions, so release builds trap deterministically instead of reading out of bounds.
String(_ bb: ByteBuffer, o: Int32) — the length prefix was read without checking o against the buffer size; corrupted offsets now resolve to an empty string.
getRoot / getPrefixedSizeRoot — a buffer smaller than a root offset crashed with an unhelpful force-unwrap, and a 1–3 byte buffer read out of bounds. Both now fail fast with descriptive messages. (The checked getCheckedRoot family is unaffected; these guards only make the documented non-checked path fail deterministically.)

FlexBuffers

FlexBuffersWriter UTF-8 lengths — add(string:) and add(key:) stored String.count (grapheme clusters) as the byte length while writing UTF-8 bytes, so any non-ASCII string or map key was silently truncated on write, and sorted-map lookups of non-ASCII keys could never match. Lengths are now utf8.count.
Value.elementWidth — the byte-width search iterated stride(from: 1, to: 8, by: 2) (i.e. 1, 3, 5, 7), but the width match (1 << bitWidth) == byteWidth can only succeed for 1, 2, 4, 8, so every offset wider than a byte fell back to .w64, bloating serialized buffers. The search now iterates 1, 2, 4, 8 like the C++ implementation.
binarySearch over map keys — initialized right = vector.count with a left <= right loop, probing index count (one past the end) whenever the lookup target was greater than every key.
getRoot — the trailing root byte width was used unvalidated (0–255) to compute the root offset, producing a negative offset for large widths; widths are now restricted to 1, 2, 4, 8 (FlexBuffersErrors.invalidByteWidth) and an unaddressable root throws sizeOfBufferIsTooSmall.
TypedVector.mapKeys — a corrupted child byte width was propagated into the key vector (and numericCast could trap on overflow); a structurally invalid key vector is now treated as empty instead of reading out of bounds.
TypedVector.compare — used strcmp against an unchecked raw buffer pointer, reading past the end of the buffer for keys without a null terminator; the comparison is now bounded by the buffer capacity.

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

swift test — 106 tests in 17 suites pass (debug).
swift test -c release — the same 106 tests pass; this configuration is the important one because it is where the previous assert-only checks were compiled out.
Two new test suites (FlatbuffersInputValidationTests, FlexBuffersInputValidationTests) add regression coverage for every fix above: an 11-case Table.compare ordering matrix, out-of-range readString matrices, corrupted-offset string inits, non-ASCII string and map-key round-trips, map lookups outside the key range, invalid root byte widths, and an elementWidth power-of-two regression.
Verified locally with Swift 6.4 on Debian 13 (x86_64).

…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.
@google-cla

google-cla Bot commented Sep 22, 2026

Copy link
Copy Markdown

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.

@github-actions github-actions Bot added the swift label Sep 22, 2026
@krishna28238-arch

Copy link
Copy Markdown
Author

i have done the cla agreement

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant