fix(relay): cap filter count on the HTTP bridge /query and /count - #4986
Open
shani-singh1 wants to merge 1 commit into
Open
fix(relay): cap filter count on the HTTP bridge /query and /count#4986shani-singh1 wants to merge 1 commit into
shani-singh1 wants to merge 1 commit into
Conversation
The WebSocket REQ/COUNT door rejects filter lists longer than `MAX_FILTERS_PER_REQ` (10) — the `max_filters` the relay advertises in NIP-11. The HTTP bridge reaches the same `query_events`/`count_events` machinery but parsed the body's filter list with no cap, so a single ~1 MB request expands into ~10^5 independent DB queries (or unbounded COUNT aggregate scans) against the shared pool — a resource-amplification DoS triggerable by any authenticated participant, one request per rate-limit tick. Enforce the same advertised cap at both bridge parse sites, before any filter becomes a query. The constant is now `pub(crate)` so the WS door and the bridge share one source of truth. Legitimate clients send 1-2 filters per query, well under the advertised limit. Adds `advertised_max_filters_matches_the_enforced_cap` (nip11.rs) pinning that the advertised `max_filters` equals the enforced constant, so the NIP-11 document, the WS door, and the bridge can't drift apart. Signed-off-by: Shani Singh <teamdeveloperworld@gmail.com>
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.
Fixes #4985.
Problem
The HTTP bridge's
POST /queryandPOST /countparse a NIP-01 filter list from the body with no cap, while the WebSocket door enforcesMAX_FILTERS_PER_REQ = 10— themax_filtersthe relay advertises in NIP-11. Each filter is an independent DB query (or, for/count, an unbounded aggregate scan), so a single ≤1 MB request expands into ~10⁵ queries against the shared pool. Full analysis + repro in #4985.Fix
Enforce the same advertised cap at both bridge parse sites — mirroring
protocol.rs, which the WS REQ/COUNT door already checks:crates/buzz-relay/src/api/bridge.rs/query: reject whenraw_filters.len() > MAX_FILTERS_PER_REQ(before any filter is turned into a query), returning400 too many filters./count: same, on the parsed filter list.crates/buzz-relay/src/protocol.rs: the constant becomespub(crate)so both doors share one source of truth (it was already the value NIP-11 advertises).Legitimate clients are unaffected: the desktop and CLI send 1–2 filters per
/query; 10 is comfortably above real usage and is exactly what the relay tells clients it accepts.Test
Added
advertised_max_filters_matches_the_enforced_cap(nip11.rs) — pins that the advertisedmax_filtersequalsMAX_FILTERS_PER_REQ, so the NIP-11 document, the WS door, and the bridge can't drift apart (the relay must never advertise a cap it doesn't enforce). The WS-side length check is already covered byprotocol.rs's existing tests.The
/query·/counthandlers themselves are only exercised by#[ignore]d Redis/Postgres integration tests, so a full end-to-end 11-filter→400 test would be infra-gated; the guard is a straight length check placed before any DB work, mirroring the tested WS path.Validation
Built on Windows with the
x86_64-pc-windows-gnutoolchain.cargo test -p buzz-relay --lib advertised_max_filters_matches_the_enforced_cap— passes.cargo fmt/clippyon the changed crate — clean.Local-build caveat (honest)
I could not compile
buzz-relaylocally: on my Windows box the only working Rust toolchain isx86_64-pc-windows-gnu, and a heavy transitive dependency (iroh-relay) fails to link under mingw here (MSVC isn't set up). buzz-relay's own source — including this change — never reaches the compiler; only the dependency graph fails. So I have rustfmt-clean (files parse) but no compiler/test run locally.The change is deliberately minimal to make that low-risk: a visibility modifier, two
if len > cap { return 400 }guards copied verbatim from the adjacentapi_error(StatusCode::BAD_REQUEST, …)pattern in the same functions, and oneassert_eq!onOption<u32>. CI (Linux) compiles and runs the test. Flagging it plainly rather than implying a green local run.