[Security][Decision] SSRF: HttpCallback scheduled task fetches an unrestricted payload-supplied URL (admin/staff-gated) #438

New Issue

2026-07-06T06:46:02Z

spikerj commented

2026-07-06 06:46:02 +00:00

Finding (needs a design decision — not blind-fixing)

SpikerSoft.EventHandlers.Scheduler/TaskRunners/HttpCallbackTaskRunner.cs executes a scheduled task by making a server-side HTTP request to a fully payload-supplied URL, with payload-supplied method, headers, and body, and no egress restriction:

var request = new HttpRequestMessage(new HttpMethod(payload.Method ?? "GET"), payload.Url);
// payload.Headers added verbatim; payload.Body as content
var response = await httpClient.SendAsync(request, ct);

Because it runs inside the scheduler worker (on the internal overlay networks), a task can target internal-only services the public can't reach — http://mongo-router:27017, the Redis nodes, http://keycloak:8080, the Docker socket proxy, etc. — i.e. a classic SSRF pivot. The response body (first 500 chars) is also echoed back in the task result, enabling blind-ish exfil.

Severity: medium (not critical)

Creating scheduled tasks is gated [Authorize(Roles = "admin,staff")] (ScheduledTasksController), so this is not anonymous/any-user — it needs an admin or staff account. Two reasons it still matters for a millions-of-kids platform:

If "staff" includes a broad population (e.g. teachers), that's a wide trust boundary for an internal-network fetch primitive.
Defense-in-depth: a compromised staff/admin session shouldn't convert into an internal-network SSRF against Mongo/Redis/Docker.

Why this is a decision, not a unilateral patch

The runner is an intentional "call a configurable URL" feature. Locking it down means answering:

Are internal callback targets ever legitimate? (internal automation vs. external-webhook-only) → determines allowlist vs. denylist.
If denylist: block loopback, RFC1918/private ranges, link-local (169.254/16), and the overlay service hostnames — or an explicit egress allowlist of approved hosts.
Should staff be able to create HttpCallback tasks at all, or only admin? (tighten the authz on this task type specifically)

Recommendation

Add an SSRF egress guard applied in the runner (resolve the host, reject private/loopback/link-local/overlay targets unless explicitly allowlisted) + optionally restrict the HttpCallback task type to admin. Happy to implement once the allowlist/denylist policy + authz scope are decided.

Scope note (rest of the SSRF surface is clean)

Swept the codebase: other outbound HTTP is config-driven infra (health checks, Gitea/Docker/MCP/GeoIP from options — not user input), and the Twilio MakeCall/SendSms URLs are fetched by Twilio (external), not our server. Also confirmed no zip-slip (no untrusted archive extract-to-disk) and no XXE (no raw XML parsing) in the same sweep.

## Finding (needs a design decision — not blind-fixing) `SpikerSoft.EventHandlers.Scheduler/TaskRunners/HttpCallbackTaskRunner.cs` executes a scheduled task by making a server-side HTTP request to a **fully payload-supplied URL**, with payload-supplied **method, headers, and body**, and **no egress restriction**: ```csharp var request = new HttpRequestMessage(new HttpMethod(payload.Method ?? "GET"), payload.Url); // payload.Headers added verbatim; payload.Body as content var response = await httpClient.SendAsync(request, ct); ``` Because it runs inside the scheduler worker (on the internal overlay networks), a task can target internal-only services the public can't reach — `http://mongo-router:27017`, the Redis nodes, `http://keycloak:8080`, the Docker socket proxy, etc. — i.e. a classic SSRF pivot. The response body (first 500 chars) is also echoed back in the task result, enabling blind-ish exfil. ## Severity: medium (not critical) Creating scheduled tasks is gated `[Authorize(Roles = "admin,staff")]` (`ScheduledTasksController`), so this is **not** anonymous/any-user — it needs an admin **or staff** account. Two reasons it still matters for a millions-of-kids platform: 1. If "staff" includes a broad population (e.g. teachers), that's a wide trust boundary for an internal-network fetch primitive. 2. Defense-in-depth: a compromised staff/admin session shouldn't convert into an internal-network SSRF against Mongo/Redis/Docker. ## Why this is a decision, not a unilateral patch The runner is an **intentional** "call a configurable URL" feature. Locking it down means answering: - **Are internal callback targets ever legitimate?** (internal automation vs. external-webhook-only) → determines allowlist vs. denylist. - If denylist: block loopback, RFC1918/private ranges, link-local (169.254/16), and the overlay service hostnames — or an explicit egress allowlist of approved hosts. - Should `staff` be able to create `HttpCallback` tasks at all, or only `admin`? (tighten the authz on this task type specifically) ## Recommendation Add an SSRF egress guard applied in the runner (resolve the host, reject private/loopback/link-local/overlay targets unless explicitly allowlisted) + optionally restrict the `HttpCallback` task type to `admin`. Happy to implement once the allowlist/denylist policy + authz scope are decided. ## Scope note (rest of the SSRF surface is clean) Swept the codebase: other outbound HTTP is config-driven infra (health checks, Gitea/Docker/MCP/GeoIP from options — not user input), and the Twilio `MakeCall`/`SendSms` URLs are fetched by Twilio (external), not our server. Also confirmed **no zip-slip** (no untrusted archive extract-to-disk) and **no XXE** (no raw XML parsing) in the same sweep.

spikerj commented

2026-07-06 15:23:59 +00:00

PR #137 up — implements the recommended egress guard as a secure-by-default, with the PR as the decision point (same pattern as the CORS fail-closed change).

SsrfEgressPolicy in SpikerSoft.Common/Http: deny-internal-by-default (loopback, RFC1918, link-local/metadata, CGNAT, 0.0.0.0/8, multicast, IPv6 equivalents, mapped-IPv6 unwrapped; unknown families fail closed).
Enforced at connect time via SocketsHttpHandler.ConnectCallback on a named client → every connection (including redirect-triggered ones) re-enters the check, closing DNS-rebinding and redirect-to-internal bypasses.
Runner requires absolute http(s) URLs (kills file:/ftp:).
Escape hatches (default off): Scheduler:HttpCallback:AllowedHosts + Scheduler:HttpCallback:AllowPrivateNetworks.
⚠️ Behavior change: existing internal-target HttpCallback tasks fail with a clear allowlist hint until configured.
37 tests (address matrix incl. boundaries + smuggling cases; no network in tests).

Remaining decision on this ticket (not in the PR): should the HttpCallback task type be restricted to admin only, or stay admin,staff? One-line change either way — say which and I'll ship it, then this closes.

**PR #137 up** — implements the recommended egress guard as a secure-by-default, with the PR as the decision point (same pattern as the CORS fail-closed change). - `SsrfEgressPolicy` in `SpikerSoft.Common/Http`: deny-internal-by-default (loopback, RFC1918, link-local/metadata, CGNAT, `0.0.0.0/8`, multicast, IPv6 equivalents, mapped-IPv6 unwrapped; unknown families fail closed). - Enforced at **connect time** via `SocketsHttpHandler.ConnectCallback` on a named client → every connection (including redirect-triggered ones) re-enters the check, closing DNS-rebinding and redirect-to-internal bypasses. - Runner requires absolute http(s) URLs (kills `file:`/`ftp:`). - Escape hatches (default off): `Scheduler:HttpCallback:AllowedHosts` + `Scheduler:HttpCallback:AllowPrivateNetworks`. - ⚠️ Behavior change: existing internal-target HttpCallback tasks fail with a clear allowlist hint until configured. - 37 tests (address matrix incl. boundaries + smuggling cases; no network in tests). **Remaining decision on this ticket** (not in the PR): should the `HttpCallback` task *type* be restricted to `admin` only, or stay `admin,staff`? One-line change either way — say which and I'll ship it, then this closes.

spikerj commented

2026-07-06 15:28:54 +00:00

Egress guard merged — PR #137 (529b462). The HttpCallback SSRF pivot is closed: deny-internal-by-default SsrfEgressPolicy enforced at connect time (blocks private/loopback/link-local/metadata/CGNAT + IPv6 equivalents; DNS-rebinding and redirect-to-internal safe), http(s)-only, with AllowedHosts / AllowPrivateNetworks escape hatches (default off). 37 tests.

The one remaining item — restricting the HttpCallback task type to admin-only vs admin,staff — is an authz-scope decision with no default I can pick for you. Closing this ticket as the SSRF vulnerability is resolved; I've filed the authz-tightening question as its own follow-up so it isn't lost.

**Egress guard merged — PR #137 (`529b462`).** The HttpCallback SSRF pivot is closed: deny-internal-by-default `SsrfEgressPolicy` enforced at connect time (blocks private/loopback/link-local/metadata/CGNAT + IPv6 equivalents; DNS-rebinding and redirect-to-internal safe), http(s)-only, with `AllowedHosts` / `AllowPrivateNetworks` escape hatches (default off). 37 tests. The one remaining item — restricting the `HttpCallback` task *type* to `admin`-only vs `admin,staff` — is an **authz-scope decision** with no default I can pick for you. Closing this ticket as the SSRF vulnerability is resolved; I've filed the authz-tightening question as its own follow-up so it isn't lost.

spikerj closed this issue

2026-07-06 15:29:00 +00:00

spikerj referenced this issue

2026-07-06 15:29:08 +00:00

[Security][Decision] Should HttpCallback scheduled-task creation be admin-only (vs admin,staff)? #442

spikerj referenced this issue

2026-07-20 22:41:07 +00:00

feature(sponsor): 'Sponsor a Journey' — family cards with shared destinations, destination chips, family detail page #749

Sign in to join this conversation.