Skip to content

fix size-prefixed buffer bounds check to avoid overflow#9056

Open
jmestwa-coder wants to merge 1 commit intogoogle:masterfrom
jmestwa-coder:verifier-size-prefixed-bound-check
Open

fix size-prefixed buffer bounds check to avoid overflow#9056
jmestwa-coder wants to merge 1 commit intogoogle:masterfrom
jmestwa-coder:verifier-size-prefixed-bound-check

Conversation

@jmestwa-coder
Copy link
Copy Markdown

Summary

Fix the bounds check in VerifierTemplate::VerifySizePrefixedBuffer() to avoid relying on addition that can wrap on 32-bit builds, and instead enforce the framing invariant directly.

Root Cause

The previous check used:

prefix + sizeof(SizeT) <= size_

This relies on unsigned addition, which can wrap on 32-bit systems. It also enforces the boundary indirectly rather than checking the invariant itself.

Change

Replaced the addition-based check with:

prefix <= size_ - sizeof(SizeT)

This:

  • avoids potential wraparound
  • directly enforces the size-prefixed buffer invariant
  • does not change behavior for valid inputs

Invariant

For size-prefixed buffers, the declared payload size must fit within the remaining bytes after the prefix.

Impact

  • Prevents acceptance of malformed size-prefixed buffers under edge conditions
  • Improves correctness of verifier boundary checks
  • No behavior change for valid buffers

Tests

  • Added invariant-based test:
    • Mutates only the size prefix
    • Ensures prefix > remaining buffer is rejected
  • Added 32-bit-only test:
    • Documents previous wraparound behavior
  • Tests are deterministic and avoid undefined behavior

Scope

  • Limited strictly to verifier boundary logic
  • No changes to helper APIs (e.g. GetSizePrefixedBufferLength)
  • No refactoring or unrelated modifications

Notes

  • Change is minimal and follows existing coding patterns
  • No impact on code generators
  • Code formatted using clang-format

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