Repository navigation
uploadproxy: never connect a client to loopback or link-local addresses - #374
Merged
Merged
Conversation
SonarCloud (S5144, SSRF) flagged that the proxy dials whatever a client asks for. A forward proxy must, but this one runs as root: a local client could use it to reach what it couldn't reach itself (root-only firewall rules, a link-local metadata service). A dialer Control hook now checks the address actually dialled (after DNS, so names and rebinding are covered) and refuses loopback, link-local, unspecified and multicast addresses, for tunnels and inspected requests alike. Private ranges stay allowed: intranet upload destinations are legitimate. Tests opt in to local targets (their sites run on 127.0.0.1). Refs #344 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
neamul602
enabled auto-merge
November 2, 2026 10:58
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the
 |
|
This was referenced Nov 2, 2026
Merged
neamul602
pushed a commit
that referenced
this pull request
Nov 2, 2026
Both came up on #379 (dev -> main). - CodeQL go/allocation-size-overflow (high), scanworker writeMsg: the frame buffer was sized 4+len(body) with no bound when sending, although the comment said frames were capped. writeMsg now refuses a frame over the protocol's largest limit (9 MB) before allocating. - SonarCloud S5144 (SSRF) in Proxy.connect: tunnels dialled the client's CONNECT target string. The connect-time guard (#374) already refused local addresses, but out of SonarCloud's sight. vetTarget now checks the port (1-65535), resolves the host itself, keeps only allowed addresses (allowedIP, shared with guardDial) and returns an IP literal, which is what gets dialled. guardDial still checks the dialled address. Refs #344 #328 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Follow-up to #373, whose SonarCloud gate failed on S5144 (server-side request forgery) in
Proxy.connect: the proxy dials whatever host a client asks for.A forward proxy has to do that, and this one only listens on loopback. But it runs as root, so a local client could use it to reach things it couldn't reach itself: services behind root-only firewall rules, or a link-local cloud metadata service (
169.254.169.254).Change
net.Dialer.Controlhook (guardDial) checks the address actually dialled, after DNS. Names that resolve to these addresses, and DNS rebinding, are therefore covered too.Options.AllowLocalTargetsturns it off for tests only; their test sites run on127.0.0.1.Test plan
localhostby name is refused. Address table:127.0.0.1,::1,169.254.169.254,0.0.0.0,fe80::1and224.0.0.1are refused;10.x,192.168.xand public IPv4/IPv6 are allowed.Refs #344
🤖 Generated with Claude Code