fix(sync): enforce end-of-input on header receive (match the reload path and sibling sinks) - #347
Open
cafebedouin wants to merge 1 commit into
Open
cafebedouin wants to merge 1 commit into
cafebedouin wants to merge 1 commit into
Conversation
pre_validate_header parsed an inbound header but did not check that read_header consumed all the payload bytes, unlike the reload path (ergo-validation/src/header/mod.rs) and the sibling receive sinks (transaction, block-section, and NiPoPoW-proof deserializers all reject trailing bytes). Consequences: a header delivered as `canonical ++ trailing` was accepted, and because the id is blake2b256(header_bytes) over the raw bytes, the same block content could enter under a non-canonical id. Adds the missing EOF check right after read_header (matching the reload path), plus a regression test: a canonical header is accepted and a trailing-byte header is rejected. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Contributor
|
Important
This repository does not receive automatic reviews because it has fewer than 10 stars. ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Advanced Run ID: Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This change was prepared with AI assistance (Claude, model Claude Opus 4.8); every claim below was verified against the source and the fix is covered by the included regression test.
pre_validate_header(ergo-sync/src/header_proc.rs) parses an inbound header withread_headerbut does not check that the parse consumed all of the payload bytes. Two consequences:canonical_bytes ++ trailing_junkis accepted —read_headerstops at the end of the header and ignores the trailing bytes.blake2b256(header_bytes)over the raw received bytes, so the same block content entering with different trailing bytes is stored under different, non-canonical ids.This is looser than the node's own reload path (
ergo-validation/src/header/mod.rs), which already rejectsconsumed != header_bytes.len()(with a comment noting the same gap), and looser than the sibling receive sinks — the transaction deserializer (ergo-validation/src/tx/mod.rs,!r.is_empty()), block-section verify (ergo-sync/src/coordinator/section_verify.rs,r.remaining() != 0), anddeserialize_nipopow_proof(ergo-ser/src/popow_proof.rs,r.remaining() != 0) all reject trailing bytes. The Ergo reference node also rejects a trailing-byte header at parse.Fix: enforce end-of-input in
pre_validate_headerimmediately afterread_header, matching the reload path. Because trailing bytes are then rejected,header_bytesis canonical and itsblake2b256id is the canonical id — closing both the trailing-byte acceptance and the raw-vs-canonical-id divergence in one check.read_headeralready consumes a header's ownunparsed_bytesfield, so a canonical header (including v5+ headers carrying realunparsed_bytes) lands exactly at end-of-input and is not affected.Test:
ergo-sync/tests/header_receive_eof.rs— a canonical mainnet header is accepted; the same header with trailing bytes appended is rejected. Existingheader_procunit tests (7) and theheader_sync_integrationsuite (23) pass unchanged.🤖 Generated with Claude Code