Skip to content

fix(security): SSRF guard for SABnzbd addurl + bounded RSS regex - #133

Merged
thedancingdeveloper merged 2 commits into
mainfrom
fix/security-ssrf-regex
Sep 9, 2026
Merged

thedancingdeveloper merged 2 commits into
mainfrom
fix/security-ssrf-regex

Conversation

@thedancingdeveloper

Copy link
Copy Markdown
Collaborator

Fixes the two genuine code-scanning findings from the security-posture review (the rest were triaged as false-positive/accepted).

1. SSRF — SABnzbd mode=addurl (code scanning: rust/request-forgery)

The native POST /api/queue/add-url already had a strict, DNS-rebinding-safe SSRF guard, but the SABnzbd-compatible addurl (what arr/NZB360 clients hit) fetched a caller-supplied URL with no validation at all.

  • Hoisted the guard into a shared module nzb_web::fetch_guard (validate_fetch_url + build_fetch_client + read_response_bytes_limited) so both paths use one implementation.
  • handle_addurl now validates the URL (rejects non-http(s) schemes and private/reserved/loopback addresses such as 169.254.169.254), pins the connection to the validated addresses (defeats DNS rebinding), and caps the body size (it was previously an unbounded read).
  • Behaviour now matches the native path. No change to the native path — the guard code was moved, not altered.

2. RSS regex (code scanning: rust/regex-injection)

User-supplied RSS match patterns are now compiled via compile_rss_regex(), which caps the pattern length (512 B) and sets an explicit size_limit on the compiled program. The Rust regex crate is already linear-time (no catastrophic backtracking), so this is defence-in-depth against resource exhaustion, not a ReDoS fix.

Behaviour note

addurl now refuses loopback/private/LAN targets, consistent with the native URL-add path. arr clients normally use addfile (upload), and indexer download URLs are public, so this matches the project's established policy.

Tests

  • SSRF unit tests moved into fetch_guard (added link-local/metadata + non-http-scheme cases).
  • addurl unit + e2e tests now assert a loopback target is refused by the guard, while still covering GET / bare-POST / form-urlencoded dispatch and URL/field extraction.
  • Added compile_rss_regex tests (valid / invalid / over-length).
  • cargo check --workspace --all-targets --locked, clippy -D warnings, and the nzb-web/rustnzb test suites all pass.

🤖 Generated with Claude Code

https://claude.ai/code/session_016xyYMPZEyc68vBok9AkJad

Resolves the two genuine code-scanning findings from the posture review:

- request-forgery (sabnzbd_compat.rs addurl): the SABnzbd-compatible
  `mode=addurl` fetched a caller-supplied URL with no validation, while the
  native `/api/queue/add-url` path already had a strict SSRF guard. Hoist that
  guard into a shared `nzb_web::fetch_guard` module (validate_fetch_url +
  build_fetch_client + read_response_bytes_limited) and route addurl through
  it. It rejects non-http(s) schemes and private/reserved/loopback hosts (e.g.
  169.254.169.254) and pins the connection to the validated addresses to
  defeat DNS rebinding. This also caps the addurl body size (previously an
  unbounded read). Behaviour now matches the native URL-add path.

- regex-injection (RSS rules): compile user-supplied RSS match patterns via a
  new compile_rss_regex() that caps the pattern length and sets an explicit
  compiled-program size_limit. (The regex crate is already linear-time, so
  this is defence-in-depth against resource exhaustion, not ReDoS.)

No behaviour change for the native paths -- the guard code is moved, not
altered. Tests: moved the SSRF unit tests into fetch_guard (plus link-local
and non-http scheme cases); addurl unit/e2e tests now assert the loopback
target is refused by the guard while still covering GET / bare-POST /
form-urlencoded dispatch; added compile_rss_regex tests.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016xyYMPZEyc68vBok9AkJad
@thedancingdeveloper
thedancingdeveloper merged commit 73ce4b2 into main Sep 9, 2026
11 checks passed
@thedancingdeveloper
thedancingdeveloper deleted the fix/security-ssrf-regex branch September 9, 2026 05:40
thedancingdeveloper added a commit that referenced this pull request Sep 9, 2026
Maintenance and hardening release covering everything merged since v1.4.6.

Bug fixes:
- SABnzbd addfile/addurl return 200 {status:false} instead of HTTP 500 on a
  failed enqueue, so Sonarr no longer sees valid grabs as hard failures
  (#130, fixes #129).
- Priority changes no longer pause active downloads (#126).
- Resuming a paused job resets its no-progress clock (#125).
- mode=addurl works as a bare POST without a multipart body (#119).

Security:
- SSRF guard applied to the SABnzbd addurl fetch (shared, DNS-rebinding-safe);
  user RSS regex compiled with length/size limits (#133).
- Alerts-only Dependabot, dependency-review check, CodeQL security-extended
  (#132). rust + CodeQL made required checks; secret-scanning validity checks
  enabled; code-scanning backlog triaged to zero (out of band).

Dependencies: quick-xml 0.37->0.41 (parser migration), zip->8.6, tower-http
->0.7, toml->1.1, socket2->0.6, base64->0.23, rand->0.10, md-5->0.11,
sha2->0.11, rust-minor group (17), codeql-action v4.

CI: reconcile stale desktop/src-tauri/Cargo.lock (#131); runner selector
migration (#120).


Claude-Session: https://claude.ai/code/session_01L7gQssrAVUE6191dfgPZi6

Co-authored-by: thedancingdeveloper <306930456+thedancingdeveloper@users.noreply.github.com>
Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
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.

1 participant