Skip to content

Fix FlexBuffers VerifyKey() inverted null-terminator check#9019

Open
Sebasteuo wants to merge 1 commit intogoogle:masterfrom
Sebasteuo:fix-flexbuffers-verifykey
Open

Fix FlexBuffers VerifyKey() inverted null-terminator check#9019
Sebasteuo wants to merge 1 commit intogoogle:masterfrom
Sebasteuo:fix-flexbuffers-verifykey

Conversation

@Sebasteuo
Copy link
Copy Markdown

Fix FlexBuffers VerifyKey() inverted null-terminator check

VerifyKey() incorrectly returned true upon encountering a non-null byte instead of verifying that a null terminator exists within the buffer bounds.

Bug

The condition if (*p++) return true; returns true on the first non-null byte, meaning any key with at least one non-null byte passes verification regardless of whether a null terminator exists within the buffer.

Impact

After verification, calls to AsString(), AsKey(), or ToString() on a FBT_KEY reference invoke strlen() which reads past the buffer boundary (heap/stack OOB read).

Fix

Negate the condition to check for the null terminator:

-      if (*p++) return true;
+      if (!*p++) return true;

Reproduction

uint8_t poc[] = {0x00, 0x10, 0x01, 0x10, 0x01};
std::vector<uint8_t> rt;
bool valid = flexbuffers::VerifyBuffer(poc, sizeof(poc), &rt);
// valid == true (WRONG, should be false)
auto root = flexbuffers::GetRoot(poc, sizeof(poc));
auto s = root.AsString();  // OOB read via strlen()

Compile with: clang++ -fsanitize=address -I include poc.cc src/util.cpp

Found via fuzzing with AddressSanitizer.

VerifyKey() incorrectly returned true upon encountering a non-null
byte instead of verifying that a null terminator exists within the
buffer bounds. This caused the verifier to accept buffers with
non-null-terminated keys, leading to out-of-bounds reads via
strlen() in AsString(), AsKey(), and ToString().

Found via fuzzing with AddressSanitizer.
@Sebasteuo Sebasteuo requested a review from dbaileychess as a code owner April 3, 2026 18:04
@github-actions github-actions bot added the c++ label Apr 3, 2026
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