Skip to content

[SEC-V-02] sync_since builds unbounded SQL from peer-supplied heads #302

@intendednull

Description

@intendednull

Audit finding from #300 (commit 679f9fe)

Severity: medium (DoS), low (security)
Category: input validation / DoS
File: crates/storage/src/store.rs:289 (and history at :184)
Obvious fix: yes

Description

sync_since accepts heads: &HeadsSummary (originating from a peer's WorkerRequest::Sync over gossip) and concatenates one (author = ?N AND seq > ?N+1) clause per entry into a SQL string. The same pattern exists in history() for before: &HeadsSummary. There is no upper bound on heads.heads.len() before the query is built.

Impact / Threat

An attacker can stuff thousands of unique entries (within the transport's 256KB envelope cap), producing a huge query that exceeds rusqlite's bind-parameter limit (default 32766) or wastes CPU compiling a giant prepared statement. May panic / error inside rusqlite on overflow.

Suggested fix

Cap heads.heads.len() (e.g. MAX_AUTHORS_PER_SYNC = 1024) at the start of sync_since; on overflow return WorkerResponse::Denied { reason: "too many heads" }. Or rewrite to insert authors into a temp table and JOIN, which is O(1) parameters.

Verify

rg "for \(author, head\) in &heads.heads" crates/storage/src/store.rs

Metadata

Metadata

Assignees

No one assigned

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions