Skip to content

fix: stack overflow in FlexBuffers ToString via unbounded mutual recursion#9078

Open
SongTonyLi wants to merge 2 commits into
google:masterfrom
SongTonyLi:fix/issue-9074-tostring-recursion-depth
Open

fix: stack overflow in FlexBuffers ToString via unbounded mutual recursion#9078
SongTonyLi wants to merge 2 commits into
google:masterfrom
SongTonyLi:fix/issue-9074-tostring-recursion-depth

Conversation

@SongTonyLi
Copy link
Copy Markdown

Summary

Fixes #9074.

Reference::ToString and AppendToString<Vector> call each other back and forth with no depth check. A 31-byte crafted FlexBuffer that contains a self-referential vector passes VerifyBuffer fine, but then ToString recurses about 247 times and blows the stack.

Root cause

The PoC encodes a vector at offset 28 whose single element points right back to offset 28 (i.e., it references itself). The verifier doesn't catch this because its reuse-tracker has already marked offset 28 as verified on the first pass, so when it sees offset 28 again it just short-circuits and says "already checked, we're good."

Once ToString gets called on this, it enters an infinite loop:

  • Reference::ToString sees a vector, calls AppendToString<Vector>
  • AppendToString<Vector> iterates the child, calls child.ToString(...)
  • which sees a vector again, calls AppendToString<Vector>
  • ...repeat until the stack runs out

I put some trace probes into the verifier to see what was happening:

  1. VerifyRef entered at data_off=28 with off=0, indirect_off=28, type=10 (FBT_VECTOR) -- the self-reference
  2. VerifyVector ran at p_off=28 the first time (full shape check), then entered again at the same p_off=28 and exited immediately via the reuse-tracker short-circuit
  3. VerifyBuffer returned ok=1 -- accepted the cyclic buffer

Then ASan caught the stack overflow: ~247 alternating ToString/AppendToString<Vector> frames before it hit the guard page.

Fix

ToString already has a cur_indent parameter that gets incremented on every recursive call into a nested container. I added a kToStringMaxDepth = 64 constant and a check right before the map/vector/typed-vector branches: if cur_indent >= 64, just emit "..." and stop recursing. No new function parameters needed since cur_indent was already being threaded through.

Why this approach

I considered putting the depth guard inside AppendToString<Vector> instead, but that only covers vector-type recursion. The map branch in ToString also recurses (through vals[i].ToString(..., cur_indent + 1, ...)), so a guard there would still leave map recursion unbounded. Putting the check in ToString itself covers all container types in one place.

Verification

  • Before: ERROR: AddressSanitizer: stack-overflow with ~247 alternating ToString/AppendToString<Vector> frames
  • After: ToString produces truncated output (259 chars with "..." at the depth limit), exits cleanly with no crash

Regression testing

Check Before patch After patch
Build exit=0 exit=0
PoC ASan run stack-overflow crash clean exit=0, 259 chars output
PoC gdb batch ~247 frame backtrace normal exit, no crash
ASan stack-overflow signature present absent
cppcheck no findings no new findings
…rsion

ToString and AppendToString<Vector> recurse into each other with no
depth limit. A crafted 31-byte FlexBuffer with self-referential vectors
passes VerifyBuffer and then blows the stack (~247 frames).

Fix: cap recursion at depth 64 using the existing cur_indent parameter,
emitting "..." when the limit is reached.

Fixes google#9074
@github-actions github-actions Bot added the c++ label May 6, 2026
@SongTonyLi SongTonyLi marked this pull request as ready for review May 6, 2026 17:20
@SongTonyLi SongTonyLi requested a review from dbaileychess as a code owner May 6, 2026 17:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

1 participant