fix(zetesis): close indexer-fetch SSRF, injection, XML-DoS, and key-leak defects - #556
Merged
Conversation
…eak defects Adversarial audit found a cluster of security defects on the indexer-fetch path. Each is closed at its enforcement point: - SSRF DNS-rebinding TOCTOU: validate_fetch_url checked resolved IPs then discarded them; the shared client re-resolved at connect time. Add an SsrfGuardResolver (reqwest::dns::Resolve) that re-checks every resolved address at connect time and rejects disallowed ones; wire it into the shared client. Pre-check kept as defense-in-depth. - IPv4-compatible IPv6 (::/96) gap: ip_is_disallowed omitted the deprecated ::a.b.c.d embed, so [::127.0.0.1] passed. Port net_validate's exact handling (mapped -> v6 specials -> ::/96 embed). - Cardigann $raw injection: $raw inputs rendered variable expansions unescaped and spliced into the query via set_query, so a term like 'a&b=c' injected extra params. Encode $raw expansions via render_url, matching the search-path encoding; literal $raw structure untouched. - API keys leaked via tracing spans: download()/get_inner() were #[instrument] without skipping the url arg, capturing apikey= as a span field. Skip url on every secret-bearing instrumented fn in the client tree. - cf_bypass skipped the body-size cap: ByparrProxy::get read via response.json() with no cap, making max_response_body_bytes a no-op for cf_bypass indexers. Enforce the cap (Content-Length precheck + streamed counter) on the byparr envelope before buffering. - Untrusted caps XML stack overflow: parse_caps_xml deserialized into the self-referential CapsCategory, overflowing the stack during serde's recursive descent before the iterative converter ran. Bound XML nesting depth on the raw event stream before deserialization. Gate-Passed: kanon 0.1.5 +stages:fmt,check,clippy,nextest,lint sha:29a0bba2e9f4fb3081110d37bc6dc2445c23514d
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
1 participant
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.
Closes a cluster of SECURITY defects on the indexer-fetch path (deep-audit).
reqwest::dns::Resolve(SsrfGuardResolver) re-checks every resolved address against the disallow list at connect time and refuses the connection on any private/loopback/link-local hit, closing the TOCTOU between validation and the live fetch. The pre-validation is kept as defense-in-depth.ip_is_disallowednow judges the::/96embed (e.g.[::127.0.0.1]), matching paroche/net_validate.$rawinjection (injection: unescaped $raw template splice in Cardigann search URL #519) —$rawtemplate values now percent-encode variable expansions (viarender_url), so a search term with&/=can no longer inject query parameters.#[instrument]sites on the fetch/download pathskip(url)(URLs embedapikey/passkey/authkey).max_response_body_bytesvia a bounded read.Gate
kanon gate --fullgreen (1833 tests); new regression test per fix.Closes #518
Closes #519
Closes #520
Closes #521
Closes #532
Closes #550