fix: block cover downloads from private hosts and cap their size #19

Merged
jfro merged 3 commits from jtk/cover-url-ssrf into main 2026-10-18 16:22:56 +00:00
Owner

Fixes #6 (high): cover URLs from request params (add_from_lookup, scan_barcode, batch_add) went straight to Req.get, so a user could make the server fetch localhost, 169.254.169.254 or LAN admin pages; anything that looked like an image was stored and served publicly.

Stacked on #18 (critical #5). This branch includes #18's commit; the new change is the second commit, 7c140c8. Merge #18 first.

Changes (lib/fuzzy_catalog/storage.ex)

  • Resolve the host (A and AAAA, or the IP literal) and refuse if any address is loopback, private (10/8, 172.16/12, 192.168/16), link-local (169.254/16, fe80::/10), CGNAT (100.64/10), 0/8, 192.0.0/24, 198.18/15, multicast/reserved (224+), ::, ::1, ULA (fc00::/7), ff00::/8, or an IPv4-mapped / NAT64 form of those. Returns {:error, :private_address}.
  • redirect: false; redirects are followed by hand, at most 3, with the same check on every hop.
  • 5 s connect and 10 s receive timeouts, retry: false, compressed: false, and the body is streamed with a 10 MB cap ({:error, :too_large}).
  • Sync covers (store_synced_cover/1) pass allow_private: true: those URLs are built from admin-configured Audiobookshelf/BookLore servers, which are usually on the LAN.
  • Logging: host, size, type and duration; refused hosts at warning. Stopped logging BookLore cover URLs (they carry its access token in the query); the sync log shows scheme/host/path only.
  • :cover_download_req_options app env lets tests plug in Req.Test.

Known limit: DNS is resolved for the check and again by the HTTP client, so a DNS-rebinding host could still slip through. Pinning the connection to the checked IP needs custom connect options with SNI; I left that out for this release.

Tests

test/fuzzy_catalog/storage_test.exs: public host stored; 12 internal URLs refused without any request; redirect to the metadata IP refused; redirect chain followed and loop stopped; >10 MB body refused; synced LAN cover allowed; public_address?/1 cases. mix precommit: 179 tests pass.

Fixes #6 (high): cover URLs from request params (`add_from_lookup`, `scan_barcode`, `batch_add`) went straight to `Req.get`, so a user could make the server fetch `localhost`, `169.254.169.254` or LAN admin pages; anything that looked like an image was stored and served publicly. **Stacked on #18** (critical #5). This branch includes #18's commit; the new change is the second commit, 7c140c8. Merge #18 first. ## Changes (`lib/fuzzy_catalog/storage.ex`) - Resolve the host (A and AAAA, or the IP literal) and refuse if any address is loopback, private (10/8, 172.16/12, 192.168/16), link-local (169.254/16, fe80::/10), CGNAT (100.64/10), 0/8, 192.0.0/24, 198.18/15, multicast/reserved (224+), `::`, `::1`, ULA (fc00::/7), ff00::/8, or an IPv4-mapped / NAT64 form of those. Returns `{:error, :private_address}`. - `redirect: false`; redirects are followed by hand, at most 3, with the same check on every hop. - 5 s connect and 10 s receive timeouts, `retry: false`, `compressed: false`, and the body is streamed with a 10 MB cap (`{:error, :too_large}`). - Sync covers (`store_synced_cover/1`) pass `allow_private: true`: those URLs are built from admin-configured Audiobookshelf/BookLore servers, which are usually on the LAN. - Logging: host, size, type and duration; refused hosts at warning. Stopped logging BookLore cover URLs (they carry its access token in the query); the sync log shows scheme/host/path only. - `:cover_download_req_options` app env lets tests plug in `Req.Test`. Known limit: DNS is resolved for the check and again by the HTTP client, so a DNS-rebinding host could still slip through. Pinning the connection to the checked IP needs custom connect options with SNI; I left that out for this release. ## Tests `test/fuzzy_catalog/storage_test.exs`: public host stored; 12 internal URLs refused without any request; redirect to the metadata IP refused; redirect chain followed and loop stopped; >10 MB body refused; synced LAN cover allowed; `public_address?/1` cases. `mix precommit`: 179 tests pass.
fix: only download covers from http(s) URLs, never local paths
All checks were successful
CI / test (pull_request) Successful in 34s
d7fa451198
download_and_store_cover took any string that existed on disk as a local
file, so a logged-in user could post book[cover_url]=/etc/passwd and get
the file copied into the public covers directory.

- download_and_store_cover only accepts http/https URLs
- local covers (Calibre sync) go through store_synced_cover/1, which only
  reads regular files inside the configured Calibre library path
- store_cover checks the image's magic bytes and picks the type and
  extension from them, ignoring the declared type

Fixes #5
fix: block cover downloads from private hosts and cap their size
All checks were successful
CI / test (pull_request) Successful in 35s
7c140c8697
Cover URLs from request params went straight to Req.get, so a user could
make the server fetch localhost, the cloud metadata address or LAN admin
pages, and the result was stored and served publicly if it looked like
an image.

- resolve the host and refuse loopback, private, link-local, CGNAT, ULA,
  multicast and IPv4-mapped/NAT64 forms of those
- follow redirects by hand (at most 3), rechecking each hop
- 5 s connect / 10 s receive timeouts, no retries, 10 MB body cap
- sync covers (admin-configured Audiobookshelf/BookLore, often on the LAN)
  may still use private hosts
- stop logging BookLore cover URLs, which carry its access token

Fixes #6
jfro left a comment

Review by Nye (PR review / audit) — APPROVE

  • Resolution check + manual redirects (each hop rechecked, max 3), compressed: false, streamed 10 MB cap, 5s/10s timeouts and retry: false all look right.
  • allow_private is only used by store_synced_cover (admin-configured sync hosts), never on request input.
  • BookLore token is no longer logged.
  • Check and connect both use the Erlang resolver, so odd host spellings (0x7f000001 etc.) are judged the same way they're connected.

Non-blocking follow-ups (filed as one low issue):

  • (a) DNS rebinding: pin the connection to the checked IP (connect to the address, send Host/SNI for the name). Known limit, OK for this release.
  • (b) public_address? treats IPv4-compatible ::a.b.c.d (::/96, e.g. ::7f00:1) as public; add {0,0,0,0,0,0,_,_} to the blocked set.
  • (c) a == 192 and b == 0 blocks all of 192.0.0.0/16; only 192.0.0.0/24 and 192.0.2.0/24 are special. Harmless over-block.
**Review by Nye (PR review / audit) — APPROVE** - Resolution check + manual redirects (each hop rechecked, max 3), `compressed: false`, streamed 10 MB cap, 5s/10s timeouts and `retry: false` all look right. - `allow_private` is only used by `store_synced_cover` (admin-configured sync hosts), never on request input. - BookLore token is no longer logged. - Check and connect both use the Erlang resolver, so odd host spellings (`0x7f000001` etc.) are judged the same way they're connected. Non-blocking follow-ups (filed as one low issue): - (a) DNS rebinding: pin the connection to the checked IP (connect to the address, send Host/SNI for the name). Known limit, OK for this release. - (b) `public_address?` treats IPv4-compatible `::a.b.c.d` (`::/96`, e.g. `::7f00:1`) as public; add `{0,0,0,0,0,0,_,_}` to the blocked set. - (c) `a == 192 and b == 0` blocks all of 192.0.0.0/16; only 192.0.0.0/24 and 192.0.2.0/24 are special. Harmless over-block.
Merge remote-tracking branch 'origin/main' into jtk/cover-url-ssrf
All checks were successful
CI / test (pull_request) Successful in 35s
0a153bfb8e
# Conflicts:
#	lib/fuzzy_catalog/storage.ex
#	test/fuzzy_catalog/storage_test.exs
jfro merged commit 5dd20bb0d5 into main 2026-10-18 16:22:56 +00:00
jfro deleted branch jtk/cover-url-ssrf 2026-10-18 16:22:56 +00:00
Sign in to join this conversation.
No reviewers
No milestone
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference
jfro/fuzzy_catalog!19
No description provided.