Skip to content

fix(sync): validate discovery hash responses#318

Open
evan-forbes wants to merge 1 commit into
ironwood-mainfrom
codex/sync-discovery-validation
Open

fix(sync): validate discovery hash responses#318
evan-forbes wants to merge 1 commit into
ironwood-mainfrom
codex/sync-discovery-validation

Conversation

@evan-forbes

Copy link
Copy Markdown

Motivation

Closes: N/A

This fixes a DoS variant of GHSA-h9hm-m2xj-4rq9 where malformed nonempty FindBlocks responses could poison sync state. For example, a response shaped like [unknown, known, random] could cause Zebra to enqueue known hashes and then cancel future downloads when the known block download failed.

Solution

  • Validate legacy discovery hash responses before mutating download state.
  • Reject duplicate hashes, known hashes after the first unknown hash, locator echoes in advertised suffixes, and nonproductive shapes such as [R].
  • Apply the validation in both tip discovery and extension-building paths.
  • Add regression coverage for [U,K,R], duplicates, [R], and malformed extension responses.

Validation

  • cargo test -p zebrad components::sync
  • cargo fmt --all -- --check

In this environment the zebrad tests were run with CXXFLAGS='-include cstdint' to work around a local librocksdb-sys C++ include issue.

AI Disclosure

Used OpenAI Codex to audit, implement, and test this change.

@evan-forbes
evan-forbes marked this pull request as ready for review July 1, 2026 14:48
@evan-forbes
evan-forbes changed the base branch from feat/pre-release-main to ironwood-main July 1, 2026 20:43
@evan-forbes
evan-forbes force-pushed the codex/sync-discovery-validation branch 2 times, most recently from b252cda to d44bd7d Compare July 1, 2026 22:53
@evan-forbes
evan-forbes force-pushed the codex/sync-discovery-validation branch from d44bd7d to abebf4e Compare July 1, 2026 23:20

@ValarDragon ValarDragon left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

LGTM, lets merge in after Friday

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.

2 participants