Skip to content

fix(merkletree): reject non-positive segment sizes in ReadAll - #902

Open
gophersg wants to merge 1 commit into
Consensys-Incorporated:masterfrom
gophersg:master
Open

gophersg wants to merge 1 commit into
Consensys-Incorporated:masterfrom
gophersg:master

Conversation

@gophersg

@gophersg gophersg commented Oct 2, 2026 •

Copy link
Copy Markdown

Description

merkletree.Tree.ReadAll accepts a public segmentSize int with no validation.
When segmentSize == 0, io.ReadFull(r, make([]byte, 0)) returns (0, nil), so the
for loop never hits the io.EOF break and keeps pushing empty leaves forever.
When segmentSize < 0, make([]byte, segmentSize) panics with
makeslice: len out of range.

Both ReaderRoot and BuildReaderProof call ReadAll, so a non-positive segment
size (e.g. a computed/zero length from a caller) can hang the process or panic.
This is the same input-validation hardening class as #893 and #878.

This PR adds an early segmentSize <= 0 check at the top of ReadAll and returns a
clear error, so all callers fail fast.

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature
  • Breaking change
  • This change requires a documentation update

How has this been tested?

  • go test ./accumulator/merkletree/ -run TestReadAll -v
    - TestReadAllRejectsNonPositiveSegmentSize: segmentSize 0 and -1 rejected by
    ReadAll, ReaderRoot, and BuildReaderProof
    - TestReadAllValidSegmentSize: positive segmentSize still builds the expected root
  • gofmt -l clean and go vet ./accumulator/merkletree/ clean

How has this been benchmarked?

  • N/A — pure early-return on invalid input; no hot-path change.

Checklist:

  • I have performed a self-review of my code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation (N/A for this fix)
  • I have added tests that prove my fix is effective or that my feature works
  • golangci-lint does not output errors locally
  • New and existing unit tests pass locally with my changes
  • Any dependent changes have been merged and published in downstream modules

Note

Low Risk
Input validation only on an invalid edge case; valid positive segment sizes are unchanged per regression test.

Overview
Tree.ReadAll now rejects segmentSize <= 0 up front with merkletree: segment size must be positive, instead of infinite looping when size is 0 or panicking on negative make([]byte, segmentSize).

Because ReaderRoot and BuildReaderProof both call ReadAll, invalid segment sizes from callers now fail fast with that error.

New tests cover rejection for 0 and -1 on ReadAll, ReaderRoot, and BuildReaderProof, plus a regression check that a positive segment size still yields the same root as before.

Reviewed by Cursor Bugbot for commit 49b6086. Bugbot is set up for automated code reviews on this repo. Configure here.

Signed-off-by: gophersg <gopher@2980.com>
@gophersg
gophersg requested a review from a team as a code owner October 2, 2026 06:32
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.

1 participant