Skip to content

fix: add bounds checking for data offset in joiner readAtOffset#5522

Closed
nugaon wants to merge 1 commit into
masterfrom
fix/erasure-span
Closed

fix: add bounds checking for data offset in joiner readAtOffset#5522
nugaon wants to merge 1 commit into
masterfrom
fix/erasure-span

Conversation

@nugaon

@nugaon nugaon commented Jun 30, 2026

Copy link
Copy Markdown
Member

Checklist

  • I have read the coding guide.
  • My change requires a documentation update, and I have done it.
  • I have added tests to cover my changes.
  • I have filled out the description and linked the related issues.

Description

Adds bounds checking to the leaf branch of (*joiner).readAtOffset in pkg/file/joiner/joiner.go. A malformed parent intermediate-chunk span can drive the recursion into the leaf path with dataOffsetStart beyond len(data), producing an inverted slice range and a runtime panic. The offsets are now validated and rejected as ErrMalformedTrie instead of crashing the node, consistent with the existing malformed-trie handling in the same function.

Open API Spec Version Changes (if applicable)

Motivation and Context (Optional)

Reported panic, most likely caused by an externally malformed (erasure-coded) chunk whose parent span does not match the actual data:

panic: runtime error: slice bounds out of range [16384:4096]

goroutine 6366125 [running]:
github.com/ethersphere/bee/v2/pkg/file/joiner.(*joiner).readAtOffset(...)
    joiner.go:243

With a corrupted parent span, dataOffsetStart = 16384 exceeds len(data) = 4096, making lenDataToCopy negative and yielding dataOffsetEnd = 4096, so data[16384:4096] panics. Parity shard spans are GF(2⁸) combinations of data-chunk spans and are not meaningful values, so a corrupted span must be treated as malformed rather than trusted.

Related Issue (Optional)

Screenshots (if appropriate):

AI Disclosure

  • This PR contains code that has been generated by an LLM.
  • I have reviewed the AI generated code thoroughly.
  • I possess the technical expertise to responsibly review the code generated in this PR.

@bcsorvasi bcsorvasi added this to the 2026 milestone Jun 30, 2026

@janos janos left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This is a nice bounds check fix, but a regression test would be good to have. Maybe with some fixed chunks data that have wrong offesets.

@acud

acud commented Jul 3, 2026

Copy link
Copy Markdown
Contributor

@nugaon thanks, we will reimplement with a regression test as we've managed to reproduce this.

@acud acud closed this Jul 3, 2026
@acud acud deleted the fix/erasure-span branch July 3, 2026 14:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants