Skip to content

fix(web): block private hosts in S3 endpoint test#1932

Open
richiemcilroy wants to merge 2 commits into
mainfrom
security/s3-endpoint-ssrf
Open

fix(web): block private hosts in S3 endpoint test#1932
richiemcilroy wants to merge 2 commits into
mainfrom
security/s3-endpoint-ssrf

Conversation

@richiemcilroy

@richiemcilroy richiemcilroy commented Jun 19, 2026

Copy link
Copy Markdown
Member

Blocks loopback, private, link-local and reserved hosts (including the cloud metadata IP and IPv6 literals) in the S3 endpoint connectivity test to prevent server-side request forgery.

Greptile Summary

This PR adds SSRF protection to the /test S3 endpoint by blocking private, loopback, link-local, and reserved IP ranges before the S3 client is constructed, and closes the DNS-rebinding TOCTOU window by injecting a guardedLookup function into the HTTP agents that re-validates resolved addresses at connection time.

  • Introduces expandIpv6 to canonicalise IPv6 addresses — including IPv4-mapped (::ffff:) in both dotted-decimal and pure-hex form — and isBlockedIp with correct bitmask checks for fe80::/10, fc00::/7, and loopback/unspecified ranges.
  • Replaces the previous AbortController timeout with per-agent connectionTimeout/requestTimeout, and adds a 400 response for blocked endpoints before the S3Client is ever instantiated.

Confidence Score: 4/5

The SSRF protection is well-constructed and addresses the previously identified issues (fe80::/10 bitmask, ::ffff: hex form, DNS rebinding). One narrow gap remains in the IPv4-compatible IPv6 path that a determined attacker could express in a URL, though modern OS kernels don't route those addresses to IPv4 equivalents.

The dual-layer defence (pre-flight IP/hostname check + guardedLookup at connection time) is solid. The single remaining gap — IPv4-compatible IPv6 addresses with h[5]=0 — means an input like http://[::169.254.169.254] passes isBlockedIp, though practical exploitation requires OS support that modern Linux kernels don't provide.

apps/web/app/api/desktop/[...route]/s3Config.ts — specifically the isBlockedIp IPv6 branch where the IPv4-compatible (h[5]=0) case is not handled alongside the IPv4-mapped (h[5]=0xffff) case.

Important Files Changed

Filename Overview
apps/web/app/api/desktop/[...route]/s3Config.ts Adds comprehensive SSRF protection to the /test endpoint: pre-flight isBlockedEndpoint check (with proper IPv6 handling via expandIpv6 and bit-mask range checks) plus a guardedLookup that re-validates resolved IPs at connection time to close the DNS-rebinding window. One gap remains: IPv4-compatible IPv6 addresses (h[5]=0 form) bypass isBlockedIp, though modern Linux kernels don't route them to IPv4 equivalents.
Prompt To Fix All With AI
Fix the following 1 code review issue. Work through them one at a time, proposing concise fixes.

---

### Issue 1 of 1
apps/web/app/api/desktop/[...route]/s3Config.ts:138-147
IPv4-compatible IPv6 addresses (`::a.b.c.d`, where all of h[0..5] are zero) are not caught by the current filter. The WHATWG URL parser normalises `http://[::169.254.169.254]` to hostname `[::a9fe:a9fe]`, which produces h = [0,0,0,0,0,0,0xa9fe,0xa9fe] after `expandIpv6`. Because h[5]=0 (not 0xffff), it skips the IPv4-mapped branch; because h[6] ≠ 0, it skips the loopback branch; and it returns `false` (not blocked). Extending the existing condition to also test `h[5] === 0` closes the gap and reuses the same embedded-IPv4 delegation path.

```suggestion
		// ::ffff:a.b.c.d — IPv4-mapped (h[5]=0xffff), in dotted OR pure-hex form
		// (e.g. ::ffff:7f00:1 == 127.0.0.1). Also covers IPv4-compatible
		// (::a.b.c.d, h[5]=0, deprecated by RFC 4291 but still parseable — e.g.
		// ::169.254.169.254 normalised by WHATWG URL to ::a9fe:a9fe). Evaluate
		// the embedded IPv4 directly.
		if (
			h[0] === 0 &&
			h[1] === 0 &&
			h[2] === 0 &&
			h[3] === 0 &&
			h[4] === 0 &&
			(h[5] === 0xffff || h[5] === 0)
		) {
```

Reviews (3): Last reviewed commit: "fix(web): harden S3 endpoint SSRF guard ..." | Re-trigger Greptile

@polarityinc

polarityinc Bot commented Jun 19, 2026

Copy link
Copy Markdown

Paragon Review Skipped

Hi @richiemcilroy! Your Polarity credit balance is insufficient to complete this review.

Please visit https://app.paragon.run to finish your review.

Comment thread apps/web/app/api/desktop/[...route]/s3Config.ts Outdated
Comment thread apps/web/app/api/desktop/[...route]/s3Config.ts
Comment thread apps/web/app/api/desktop/[...route]/s3Config.ts
Comment thread apps/web/app/api/desktop/[...route]/s3Config.ts Outdated
@richiemcilroy

Copy link
Copy Markdown
Member Author

@greptileai please review the PR

Comment thread apps/web/app/api/desktop/[...route]/s3Config.ts Outdated
@richiemcilroy

Copy link
Copy Markdown
Member Author

@greptileai please review the PR

@superagent-security

Copy link
Copy Markdown

Superagent didn't find any vulnerabilities or security issues in this PR.

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