Missing Null-Terminator Content Check in VerifyTerminator()
- Dominant language
- C++
- Stars
- 26.5k
- Forks
- 3.7k
- PR merge metrics
- No merged PRs in 30d
Description
## Description
While auditing the FlexBuffers verifier, I discovered a bug in `flexbuffers::Verifier::VerifyTerminator()`. The function is intended to ensure that an `FBT_STRING` value has a valid null terminator at offset `size`. However, the current implementation only verifies that the byte at that offset lies inside the buffer; it does not check that the byte equals `0x00`.
```cpp
// include/flatbuffers/flexbuffers.h:
bool VerifyTerminator(const String& s) {
return VerifyFromPointer(reinterpret_cast(s.c_str()),
s.size() + 1);
}
```
As a result, `VerifyBuffer()` incorrectly accepts FlexBuffers whose strings are not actually null-terminated. A subsequent call to `AsString().c_str()` followed by `strlen()` or `strcmp()` reads past the verified region, resulting in a **Heap Out-Of-Bounds Read** (CWE-125).
This is a sibling bug to the recently fixed logic inversion in `VerifyKey()` (Issue #9072, commit `a6979fe`), sharing the exact same vulnerability primitive. The non-flex `VerifyString()` properly checks the terminator content (`Check(buf_[end] == '\0')`).
## Impact
Any application parsing untrusted FlexBuffers and using standard C-style string APIs on the parsed strings is vulnerable to Heap OOB Reads, leading to information disclosure or Denial of Service (crashes).
## Proposed Fix
I have submitted a Pull Request that aligns `VerifyTerminator()` with the non-flex verifier by adding the missing content check, alongside a regression test:
```cpp
bool VerifyTerminator(const String& s) {
const uint8_t* p = reinterpret_cast(s.c_str());
return VerifyFromPointer(p, s.size() + 1) && p[s.size()] == 0;
}
```
Contributor guide
Assessment
This issue has not been assessed yet.