Skip to content

Commit 86eea65

Browse files
molty3000molty3000
committed
fix(security): ClassifyURL SSRF bypass — proper IP parsing replaces string prefixes
The old ClassifyURL used string prefix matching (HasPrefix on rawURL) to detect private/loopback IPs. This was trivially bypassable with representations that browsers accept but don't start with the expected prefix string: http://0 → 0.0.0.0 http://0177.0.0.1 → 127.0.0.1 (octal) http://2130706433 → 127.0.0.1 (decimal integer) http://0x7f000001 → 127.0.0.1 (hex) http://127.1 → 127.0.0.1 (short form) http://0x0.0x0.0x0.0x0 → 0.0.0.0 (hex dotted) Fix: - Parse URL with net/url to extract the host - Use parseBrowserIP() — implements inet_aton-style parsing (1-4 parts, octal/hex/decimal per part, single-integer, short forms) that matches what browsers and curl accept - Then classify with net.IP.IsLoopback(), .IsPrivate(), .IsUnspecified(), .IsLinkLocalUnicast() - Also harden hostname checks: metadata endpoints, *.internal, *.local, *.docker.internal Fixes H2 from security audit.
1 parent 88ecfdd commit 86eea65

2 files changed

Lines changed: 115 additions & 7 deletions

File tree

‎internal/danger/classifier.go‎

Lines changed: 105 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -9,8 +9,11 @@ package danger
99

1010
import (
1111
"fmt"
12+
"net"
13+
"net/url"
1214
"os"
1315
"path/filepath"
16+
"strconv"
1417
"strings"
1518
)
1619

@@ -84,20 +87,115 @@ func ClassifyPath(path string) RiskClass {
8487

8588
// ClassifyURL returns a RiskClass for a browser URL.
8689
// Internal IPs → system_write; external → network_egress.
90+
// Uses proper IP parsing (handles decimal, octal, hex, IPv6 compressed,
91+
// short forms like 127.1, and all other representations that browsers
92+
// accept via inet_aton-style parsing) instead of string prefix matching
93+
// which was trivially bypassable.
8794
func ClassifyURL(rawURL string) RiskClass {
88-
for _, prefix := range []string{
89-
"http://127.0.0.1", "http://localhost", "http://10.",
90-
"http://172.", "http://192.168.", "http://[::1]",
91-
"https://127.0.0.1", "https://localhost",
92-
"https://10.", "https://172.", "https://192.168.",
93-
} {
94-
if strings.HasPrefix(rawURL, prefix) {
95+
u, err := url.Parse(rawURL)
96+
if err != nil || u.Scheme == "" || u.Host == "" {
97+
return NetworkEgress // can't parse — don't block, but will fail at fetch time
98+
}
99+
100+
host := u.Hostname()
101+
102+
// Try as an IP address — uses browser-compatible parsing that handles
103+
// decimal (127.0.0.1), octal (0177.0.0.1), hex (0x7f000001),
104+
// mixed (127.0x1), short (127.1), single-integer (2130706433),
105+
// IPv6 compressed ([::1]), IPv4-mapped IPv6, etc.
106+
if ip := parseBrowserIP(host); ip != nil {
107+
if ip.IsLoopback() || ip.IsPrivate() || ip.IsLinkLocalUnicast() || ip.IsLinkLocalMulticast() {
108+
return SystemWrite
109+
}
110+
if ip.IsUnspecified() {
95111
return SystemWrite
96112
}
113+
return NetworkEgress
114+
}
115+
116+
// Hostname-based: check well-known private hostnames
117+
hostLower := strings.ToLower(host)
118+
switch hostLower {
119+
case "localhost", "localhost.localdomain", "localhost6", "localhost6.localdomain6",
120+
"ip6-localhost", "ip6-loopback":
121+
return SystemWrite
122+
}
123+
124+
// *.local (mDNS) resolves to link-local
125+
if strings.HasSuffix(hostLower, ".local") {
126+
return SystemWrite
127+
}
128+
129+
// Common cloud metadata endpoints (SSRF targets)
130+
if hostLower == "169.254.169.254" || hostLower == "[fd00:ec2::254]" ||
131+
hostLower == "metadata.google.internal" ||
132+
hostLower == "metadata.internal" ||
133+
strings.HasSuffix(hostLower, ".internal") {
134+
return SystemWrite
135+
}
136+
137+
// Docker internal hostnames
138+
if strings.HasSuffix(hostLower, ".docker.internal") {
139+
return SystemWrite
97140
}
141+
98142
return NetworkEgress
99143
}
100144

145+
// parseBrowserIP parses an IP address using the same rules browsers use
146+
// (inet_aton-style), handling representations that Go's net.ParseIP doesn't:
147+
// - Octal: 0177.0.0.1
148+
// - Hex: 0x7f000001, 0x0.0x0.0x0.0x0
149+
// - Integer: 2130706433
150+
// - Short: 127.1
151+
func parseBrowserIP(host string) net.IP {
152+
// Try standard parse first (handles IPv6, dotted decimal, etc.)
153+
if ip := net.ParseIP(host); ip != nil {
154+
return ip
155+
}
156+
157+
// Try inet_aton-style parsing for IPv4 with non-standard representations
158+
parts := strings.Split(host, ".")
159+
if len(parts) < 1 || len(parts) > 4 {
160+
return nil
161+
}
162+
163+
var nums []uint32
164+
for _, p := range parts {
165+
var val uint64
166+
var err error
167+
switch {
168+
case strings.HasPrefix(p, "0x") || strings.HasPrefix(p, "0X"):
169+
val, err = strconv.ParseUint(p[2:], 16, 32)
170+
case strings.HasPrefix(p, "0") && len(p) > 1:
171+
// Only octal if it starts with 0 and has more digits
172+
// Single "0" is just decimal zero
173+
val, err = strconv.ParseUint(p[1:], 8, 32)
174+
default:
175+
val, err = strconv.ParseUint(p, 10, 32)
176+
}
177+
if err != nil || val > 0xFFFFFFFF {
178+
return nil
179+
}
180+
nums = append(nums, uint32(val))
181+
}
182+
183+
switch len(nums) {
184+
case 1:
185+
// Single number: full 32-bit address
186+
return net.IPv4(byte(nums[0]>>24), byte(nums[0]>>16), byte(nums[0]>>8), byte(nums[0]))
187+
case 2:
188+
// a.b: a = high byte, b = remaining 24 bits
189+
return net.IPv4(byte(nums[0]), byte(nums[1]>>16), byte(nums[1]>>8), byte(nums[1]))
190+
case 3:
191+
// a.b.c: a, b = high bytes, c = remaining 16 bits
192+
return net.IPv4(byte(nums[0]), byte(nums[1]), byte(nums[2]>>8), byte(nums[2]))
193+
case 4:
194+
return net.IPv4(byte(nums[0]), byte(nums[1]), byte(nums[2]), byte(nums[3]))
195+
}
196+
return nil
197+
}
198+
101199
// ── Config ─────────────────────────────────────────────────────────────
102200

103201
// DangerousConfig defines how dangerous operations are handled.

‎internal/danger/classifier_test.go‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -996,6 +996,16 @@ func TestClassifyURL_InternalIPs(t *testing.T) {
996996
{"https://10.0.0.5", SystemWrite},
997997
{"https://172.20.0.1", SystemWrite},
998998
{"https://192.168.0.1", SystemWrite},
999+
// Bypass vectors that the old string-prefix classifier missed:
1000+
{"http://0", SystemWrite}, // 0.0.0.0
1001+
{"http://0177.0.0.1", SystemWrite}, // octal for 127.0.0.1
1002+
{"http://2130706433", SystemWrite}, // decimal for 127.0.0.1
1003+
{"http://0x7f000001", SystemWrite}, // hex for 127.0.0.1
1004+
{"http://127.1", SystemWrite}, // shorthand for 127.0.0.1
1005+
{"http://0x0.0x0.0x0.0x0", SystemWrite}, // hex dotted
1006+
{"http://[::0:0:0:1]", SystemWrite}, // alt IPv6 loopback
1007+
{"http://169.254.169.254", SystemWrite}, // metadata endpoint
1008+
{"http://metadata.google.internal", SystemWrite}, // GCP metadata
9991009
}
10001010
for _, tt := range tests {
10011011
t.Run(tt.url, func(t *testing.T) {

0 commit comments

Comments
 (0)