Skip to content

Bound FlexBuffers verifier work for shared references - #9197

Open
carrerasdarren-cell wants to merge 1 commit into
google:masterfrom
carrerasdarren-cell:security/flexbuffers-verifier-work-limit
Open

Bound FlexBuffers verifier work for shared references#9197
carrerasdarren-cell wants to merge 1 commit into
google:masterfrom
carrerasdarren-cell:security/flexbuffers-verifier-work-limit

Conversation

@carrerasdarren-cell

Copy link
Copy Markdown
Contributor

Summary

  • bound aggregate FlexBuffers vector traversal and key scanning work
  • keep highly shared DAGs supported through the existing reuse tracker
  • add regression coverage for the bounded default and tracker-enabled paths

Fixes #9127.

Problem

The public OSS-Fuzz testcase for issue 522117389 contains a 131,069-byte
FlexBuffer nested in a FlatBuffer. Repeated references cause the default
verifier to revisit the same 1,028-element key vector and rescan long key
suffixes many times.

On current master, a release build took 8.72 seconds in Verifier and another
8.72 seconds in SizeVerifier on the testcase. The ASan/UBSan reproducer was
still running after 60 seconds. Sampling placed essentially all CPU time in
Verifier::VerifyKey through the repeated shared vector.

The existing reuse tracker avoids this amplification, but it is optional and
the default verifier is commonly invoked on untrusted buffers without one.

Fix

Account for each traversed vector element and scanned key byte. The default
budget is proportional to buffer size * max depth, with saturation on
overflow. This bounds default verification while leaving room for ordinary
sharing. Callers that expect heavily shared DAGs can continue to supply the
existing reuse tracker; repeated values then short-circuit before consuming
additional work.

Key termination checks use memchr bounded by the remaining work budget, so a
single long key cannot overrun the limit before it is checked.

Validation

  • flattests: ALL TESTS PASSED
  • ASan/UBSan verifier_fuzzer reproducer: clean exit in 0.26 seconds
  • regression: a builder-generated shared-key DAG is rejected by the bounded
    default and accepted when the reuse tracker is supplied
  • git clang-format --diff: clean
  • git diff --check: clean

Cap aggregate vector traversal and key scanning relative to buffer size and maximum depth so shared references cannot amplify default verification into an excessive CPU cost. Keep heavily shared DAGs supported through the existing reuse tracker and add regression coverage for both paths.
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.

OSS-Fuzz issue 522117389

1 participant