Finding
The enqueue_download SSRF guard (crates/paroche/src/net_validate.rs, added for #373) is a strong first line but has three residual gaps that a determined caller could exploit. Severity is reduced because #373 also gates enqueue to admin-only, so these are defense-in-depth hardening, not an open member-level hole.
- DNS-rebinding TOCTOU. The guard resolves the host and checks the resolved IPs at enqueue time, but the download engine re-resolves at fetch time. An attacker controlling DNS can pass validation with a public IP, then rebind to an internal address before the fetch.
- IPv4-compatible IPv6.
ip_is_disallowed reduces IPv4-mapped IPv6 (::ffff:a.b.c.d) to its embedded v4, but not the deprecated IPv4-compatible form (::a.b.c.d, ::/96). ::127.0.0.1 is not classified as loopback and would pass. (Low practical risk — the form is deprecated and rarely routed to localhost — but it is an inconsistency.)
- Magnet tracker hostnames are not DNS-resolved.
validate_magnet_trackers rejects IP-literal and localhost trackers but does not resolve tracker domains, a deliberate asymmetry with the http(s) path.
Evidence
crates/paroche/src/net_validate.rs: validate_fetch_host resolves + checks (good); ip_is_disallowed uses to_ipv4_mapped only; validate_magnet_trackers has a WHY comment documenting the no-resolve choice.
Why this matters
The rebinding gap is the material one: it makes the http(s) IP check bypassable in principle. Full closure requires pinning the validated IP through to the fetcher so validate-time and fetch-time targets cannot diverge — a cross-crate change touching the download-execution path (ergasia/syntaxis), which is why it was scoped out of the #373 fix.
Desired correction
- Pin the validated IP address (or a validated-resolver handle) from
net_validate through to the fetcher, so the fetch connects to exactly the address that passed the check (closes rebinding).
- Reduce IPv4-compatible IPv6 to its embedded v4 for classification (after loopback/unspecified checks so
::1/:: are not mis-mapped).
- Decide and document the magnet-tracker resolution policy (resolve-and-check vs. accept-as-third-party), consistently with the fetch path.
Done when: a rebinding test (validate returns public, fetch target is pinned and cannot reach an internal address); ::127.0.0.1 and NAT64-embedded private addresses are rejected; magnet-tracker policy is explicit and tested.
Finding
The
enqueue_downloadSSRF guard (crates/paroche/src/net_validate.rs, added for #373) is a strong first line but has three residual gaps that a determined caller could exploit. Severity is reduced because #373 also gates enqueue to admin-only, so these are defense-in-depth hardening, not an open member-level hole.ip_is_disallowedreduces IPv4-mapped IPv6 (::ffff:a.b.c.d) to its embedded v4, but not the deprecated IPv4-compatible form (::a.b.c.d,::/96).::127.0.0.1is not classified as loopback and would pass. (Low practical risk — the form is deprecated and rarely routed to localhost — but it is an inconsistency.)validate_magnet_trackersrejects IP-literal andlocalhosttrackers but does not resolve tracker domains, a deliberate asymmetry with the http(s) path.Evidence
crates/paroche/src/net_validate.rs:validate_fetch_hostresolves + checks (good);ip_is_disallowedusesto_ipv4_mappedonly;validate_magnet_trackershas aWHYcomment documenting the no-resolve choice.Why this matters
The rebinding gap is the material one: it makes the http(s) IP check bypassable in principle. Full closure requires pinning the validated IP through to the fetcher so validate-time and fetch-time targets cannot diverge — a cross-crate change touching the download-execution path (ergasia/syntaxis), which is why it was scoped out of the #373 fix.
Desired correction
net_validatethrough to the fetcher, so the fetch connects to exactly the address that passed the check (closes rebinding).::1/::are not mis-mapped).Done when: a rebinding test (validate returns public, fetch target is pinned and cannot reach an internal address);
::127.0.0.1and NAT64-embedded private addresses are rejected; magnet-tracker policy is explicit and tested.