Skip to content

fix(storage): add bounds validation in Footer::Parse#2171

Open
jfeng18 wants to merge 1 commit into
antgroup:mainfrom
jfeng18:fix/serialization-bounds-check
Open

fix(storage): add bounds validation in Footer::Parse#2171
jfeng18 wants to merge 1 commit into
antgroup:mainfrom
jfeng18:fix/serialization-bounds-check

Conversation

@jfeng18

@jfeng18 jfeng18 commented Jun 8, 2026

Copy link
Copy Markdown

Summary

Add bounds validation in Footer::Parse to prevent heap out-of-bounds read on corrupted/truncated serialized index files.

Fixes: #2170

Changes

  • Validate length >= 20 before reading inner fields (minimum overhead: 8B magic + 8B metadata_length + 4B checksum)
  • Validate metadata_string_length <= length - 20 before constructing the metadata string
  • Remove unused string_buffer allocation (dead code)

Test plan

  • Footer tests: 21 assertions passed
  • Full unit tests: 441 cases / 85M assertions passed (release)
  • make fmt clean

🤖 Generated with Claude Code

…OOB read

Validate that length >= 20 bytes (8B magic + 8B metadata_length + 4B
checksum minimum) and that metadata_string_length does not exceed the
available buffer before constructing the metadata string or copying the
checksum. Without these checks, a corrupted or truncated serialized file
could cause a heap out-of-bounds read.

Also removes unused string_buffer allocation (dead code).

Signed-off-by: 水芝 <fengjiangtian.fjt@alibaba-inc.com>
Assisted-by: Claude:claude-opus-4-6
@jfeng18

jfeng18 commented Jun 8, 2026

Copy link
Copy Markdown
Author

Hi maintainers, could you please add kind/bug and version/1.0 labels? I don't have label permissions as a fork contributor. Thanks!

@mergify

mergify Bot commented Jun 8, 2026

Copy link
Copy Markdown
Contributor

Merge Protections

Your pull request matches the following merge protections and will not be merged until they are valid.

🔴 Require kind label

Waiting for

  • label~=^kind/
This rule is failing.
  • label~=^kind/

🔴 Require version label

Waiting for

  • label~=^version/
This rule is failing.
  • label~=^version/

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request introduces safety checks in Footer::Parse within src/storage/serialization.cpp to prevent potential out-of-bounds memory accesses. Specifically, it ensures that the length is at least 20 bytes and that the metadata_string_length does not exceed the available buffer size. It also removes an unused string_buffer variable. There are no review comments, and I have no feedback to provide.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

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.

fix(storage): heap out-of-bounds read in Footer::Parse on corrupted files

1 participant