Skip to content

fix: security and correctness audit — 17-file, multi-language hardening#8990

Open
canuk40 wants to merge 1 commit intogoogle:masterfrom
canuk40:fix/security-and-correctness-audit
Open

fix: security and correctness audit — 17-file, multi-language hardening#8990
canuk40 wants to merge 1 commit intogoogle:masterfrom
canuk40:fix/security-and-correctness-audit

Conversation

@canuk40
Copy link
Copy Markdown

@canuk40 canuk40 commented Mar 22, 2026

Addresses findings from a comprehensive bughunt across C++, Java, Kotlin, Python, TypeScript, Rust, and project infrastructure.

C++ core (flatbuffer_builder.h, verifier.h, flexbuffers.h, idl.h)

  • C-01: FieldIndexToOffset — replace stripped ASSERT with a real bounds check; return 0 (field-not-present sentinel) on overflow instead of UB
  • H-03: flexbuffers::GetRoot — reject buffers < 3 bytes and byte_width larger than remaining data; return a null Reference instead of reading OOB
  • H-05: VerifyTableStart — require vsize >= 2*sizeof(voffset_t); a zero-size vtable previously made all Required fields appear falsely present
  • H-14/C-10: VerifierTemplate constructor — record validity in valid_ flag instead of relying on a FLATBUFFERS_ASSERT stripped in release builds; VerifyBuffer() checks valid_ before proceeding

Parser (src/idl_parser.cpp, include/flatbuffers/idl.h)

  • C-04: compareFieldDefs — null-check Lookup("id") result before dereferencing inside std::sort; a null deref inside a comparator is undefined behaviour
  • C-05: StructDef::Deserialize — bounds-check field id() < vector size before using it as an index; prevents OOB write with crafted .bfbs binary schemas
  • H-04: ParseHash — null-check every FindHashFunction{16,32,64} return before calling through the pointer; return an error for unknown hash names
  • H-07: RPCCall::Deserialize — null-check call->request() and call->response() before chaining ->name()->str(); malformed schemas no longer crash
  • C-11: union type lookahead — add union_type_scan_count_ budget (max_tables * 4) to prevent O(N×M) DoS from adversarial JSON schemas

Reflection (src/reflection.cpp)

  • C-02: ForAllFields reverse iteration — off-by-one: was size()-i+1 (reads one past the end on first iteration), now size()-1-i

Rust (rust/flexbuffers/src/builder/map.rs, Cargo.toml)

  • C-12: sort_map_by_keys duplicate-key panic — replaced unconditional panic! with a debug_assert!; release builds no longer terminate the process on untrusted input with duplicate map keys
  • H-21: num_enum 0.5.1 → 0.7 (fixes soundness hole in TryFromPrimitive)

Java (java/pom.xml, FlatBufferBuilder.java, FlexBuffers.java, FlexBuffersBuilder.java)

  • H-11: Replace 8 bare assert statements with explicit throws so checks fire in production JVMs (which have assertions disabled by default): IllegalStateException / IllegalArgumentException / IndexOutOfBoundsException
  • H-18: junit 4.13.1 → 4.13.2 (CVE-2020-15250)
  • H-19: maven-release-plugin 2.5.3 → 3.0.1
  • H-20: autoReleaseAfterClose true → false (prevent accidental auto-publish)

Kotlin (FlatBufferBuilder.kt)

  • C-08: required() — getShort().toInt() sign-extends uint16 vtable offsets

    = 32768 to negative values; use .toUShort().toInt() for correct comparison

Python (python/flatbuffers/table.py, flexbuffers.py)

  • H-12: Replace 4 bare assert statements with explicit raises so checks survive python -O (optimized mode strips all assert statements): TypeError, ValueError, OverflowError

TypeScript (ts/flexbuffers/reference-util.ts)

  • C-09: indirect() — readUInt returns bigint for 64-bit width; as number was a type-only cast leaving bigint at runtime, causing TypeError on arithmetic; wrap in Number() for correct subtraction

Infrastructure (.github/dependabot.yml)

  • H-22: Add Dependabot coverage for npm, cargo (×2), maven, gradle, pip, pub — previously only github-actions was monitored

Thank you for submitting a PR!

Please delete this standard text once you've created your own description.

If you make changes to any of the code generators (src/idl_gen*) be sure to
build your project, as it will generate code based on the changes. If necessary
the code generation script can be directly run (scripts/generate_code.py),
requires Python3. This allows us to better see the effect of the PR.

If your PR includes C++ code, please adhere to the
Google C++ Style Guide,
and don't forget we try to support older compilers (e.g. VS2010, GCC 4.6.3),
so only some C++11 support is available.

For any C++ changes, please make sure to run sh scripts/clang-format-git.sh

Include other details as appropriate.

Thanks!

@canuk40 canuk40 requested a review from dbaileychess as a code owner March 22, 2026 16:29
@github-actions github-actions bot added python java c++ rust typescript codegen Involving generating code from schema kotlin CI Continuous Integration labels Mar 22, 2026
@google-cla
Copy link
Copy Markdown

google-cla bot commented Mar 22, 2026

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.

@canuk40 canuk40 force-pushed the fix/security-and-correctness-audit branch from 009d53b to 2b24079 Compare March 22, 2026 16:40
Addresses findings from a comprehensive bughunt across C++, Java, Kotlin,
Python, TypeScript, Rust, and project infrastructure.

## C++ core (flatbuffer_builder.h, verifier.h, flexbuffers.h, idl.h)
- C-01: FieldIndexToOffset — replace stripped ASSERT with a real bounds check;
  return 0 (field-not-present sentinel) on overflow instead of UB
- H-03: flexbuffers::GetRoot — reject buffers < 3 bytes and byte_width larger
  than remaining data; return a null Reference instead of reading OOB
- H-05: VerifyTableStart — require vsize >= 2*sizeof(voffset_t); a zero-size
  vtable previously made all Required fields appear falsely present
- H-14/C-10: VerifierTemplate constructor — record validity in valid_ flag
  instead of relying on a FLATBUFFERS_ASSERT stripped in release builds;
  VerifyBuffer() checks valid_ before proceeding

## Parser (src/idl_parser.cpp, include/flatbuffers/idl.h)
- C-04: compareFieldDefs — null-check Lookup("id") result before dereferencing
  inside std::sort; a null deref inside a comparator is undefined behaviour
- C-05: StructDef::Deserialize — bounds-check field id() < vector size before
  using it as an index; prevents OOB write with crafted .bfbs binary schemas
- H-04: ParseHash — null-check every FindHashFunction{16,32,64} return before
  calling through the pointer; return an error for unknown hash names
- H-07: RPCCall::Deserialize — null-check call->request() and call->response()
  before chaining ->name()->str(); malformed schemas no longer crash
- C-11: union type lookahead — add union_type_scan_count_ budget
  (max_tables * 4) to prevent O(N×M) DoS from adversarial JSON schemas

## Reflection (src/reflection.cpp)
- C-02: ForAllFields reverse iteration — off-by-one: was size()-i+1 (reads one
  past the end on first iteration), now size()-1-i

## Rust (rust/flexbuffers/src/builder/map.rs, Cargo.toml)
- C-12: sort_map_by_keys duplicate-key panic — replaced unconditional panic!
  with a debug_assert!; release builds no longer terminate the process on
  untrusted input with duplicate map keys
- H-21: num_enum 0.5.1 → 0.7 (fixes soundness hole in TryFromPrimitive)

## Java (java/pom.xml, FlatBufferBuilder.java, FlexBuffers.java, FlexBuffersBuilder.java)
- H-11: Replace 8 bare `assert` statements with explicit throws so checks fire
  in production JVMs (which have assertions disabled by default):
  IllegalStateException / IllegalArgumentException / IndexOutOfBoundsException
- H-18: junit 4.13.1 → 4.13.2 (CVE-2020-15250)
- H-19: maven-release-plugin 2.5.3 → 3.0.1
- H-20: autoReleaseAfterClose true → false (prevent accidental auto-publish)

## Kotlin (FlatBufferBuilder.kt)
- C-08: required() — getShort().toInt() sign-extends uint16 vtable offsets
  >= 32768 to negative values; use .toUShort().toInt() for correct comparison

## Python (python/flatbuffers/table.py, flexbuffers.py)
- H-12: Replace 4 bare `assert` statements with explicit raises so checks
  survive `python -O` (optimized mode strips all assert statements):
  TypeError, ValueError, OverflowError

## TypeScript (ts/flexbuffers/reference-util.ts)
- C-09: indirect() — readUInt returns bigint for 64-bit width; `as number` was
  a type-only cast leaving bigint at runtime, causing TypeError on arithmetic;
  wrap in Number() for correct subtraction

## Infrastructure (.github/dependabot.yml)
- H-22: Add Dependabot coverage for npm, cargo (×2), maven, gradle, pip, pub
  — previously only github-actions was monitored
@canuk40 canuk40 force-pushed the fix/security-and-correctness-audit branch from 2b24079 to 7ccbd0b Compare March 22, 2026 16:42
@canuk40
Copy link
Copy Markdown
Author

canuk40 commented Mar 22, 2026

@dbaileychess

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

Labels

c++ CI Continuous Integration codegen Involving generating code from schema java kotlin python rust typescript

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant