Skip to content

Commit 4c91aea

Browse files
authored
fix: block RFC 6598 CGNAT and RFC 2544 benchmark IPs (M-10) (#67)
Go's net.IP.IsPrivate() does not cover 100.64.0.0/10 (CGNAT / Tailscale) or 198.18.0.0/15 (benchmark testing). Add both ranges to internal/danger.IsBlockedIP so the policy gate and SSRF dial guard block them consistently. Changes: - internal/danger/classifier.go: IsBlockedIP now checks extraBlockedNets for 100.64.0.0/10 and 198.18.0.0/15. - Updated TestIsBlockedIP to expect these ranges as blocked. - Updated docs/SECURITY.md and AGENTS.md.
1 parent 1f30126 commit 4c91aea

4 files changed

Lines changed: 43 additions & 9 deletions

File tree

AGENTS.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -115,7 +115,7 @@ Layered prompt-injection / approval-fatigue defenses. Full reference: [docs/SECU
115115
- **Approver friction** (`internal/danger/approver.go`, `cmd/odek/wsapprover.go`) — both TTYApprover and WSApprover engage friction mode after 3 approvals of the same class in 60s: require typing literal `approve`, 1.5s pause. Trust-class shortcut disabled for `destructive` + `blocked` regardless.
116116
- **Danger classifier bypass resistance** (`internal/danger/classifier.go`) — `normalize()` pre-processes: expand `$IFS` / `${IFS}`, extract `$(...)` / `` `...` `` substitutions, strip `command` / `exec` / `builtin` wrappers, collapse unquoted backslashes, basename absolute paths. `awk`/`gawk`, `sed` (`e` command / `-f`), and editors (`vi`/`vim`/`nvim`/`emacs`/`ed`/`ex`) are classified as `code_execution` when given a script or file operand, closing `awk 'BEGIN{system(...)}'`, `sed 's///e'`, and editor `!` shell escapes. Regression suite in `classifier_bypass_test.go`.
117117
- **WebSocket CSRF token** (`cmd/odek/serve.go`) — `odek serve` issues a random 256-bit token at startup and requires it on `/ws` via cookie, `X-Odek-Ws-Token` header, or `odek.<token>` subprotocol. The token is no longer embedded in every `GET /` response; it is only delivered when the request includes the correct `?token=` query parameter (Jupyter-style), and a non-loopback bind prints a loud warning. The localhost origin check remains as defense-in-depth.
118-
- **SSRF / DNS-rebinding dial guard** (`cmd/odek/ssrf_guard.go`) — `browser`, `http_batch`, and `web_search` resolve hostnames at dial time and refuse internal IPs, then pin the dial to the validated IP. Operator-configured backends (e.g. `web_search.base_url`) are added to an allowlist so container-internal services such as SearXNG remain reachable.
118+
- **SSRF / DNS-rebinding dial guard** (`cmd/odek/ssrf_guard.go` + `internal/danger/classifier.go`) — `browser`, `http_batch`, and `web_search` resolve hostnames at dial time and refuse internal IPs (loopback, RFC1918, RFC4193, link-local, RFC 6598 CGNAT `100.64.0.0/10`, RFC 2544 benchmark `198.18.0.0/15`, and unspecified), then pin the dial to the validated IP. Operator-configured backends (e.g. `web_search.base_url`) are added to an allowlist so container-internal services such as SearXNG remain reachable.
119119
- **REST API CSRF protection** (`cmd/odek/serve.go::requireLocalOrigin`) — state-changing HTTP endpoints (POST/PUT/PATCH/DELETE) require a localhost origin or no Origin header, and static responses set `X-Frame-Options: DENY` + `Content-Security-Policy: frame-ancestors 'none'` to block clickjacking.
120120
- **Browser history cap** (`cmd/odek/browser_tool.go`) — navigation history is capped at 50 snapshots to prevent memory DoS from repeated `browser_navigate` calls.
121121
- **Browser element cap** (`cmd/odek/browser_tool.go`) — the number of interactive elements extracted per page is capped at 500 so a hostile page cannot OOM the agent with thousands of links or buttons.

docs/SECURITY.md

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -391,6 +391,12 @@ client := &http.Client{
391391

392392
There is no user-facing allowlist config field today; the list is derived from each tool's own operator-controlled `base_url`. If you need a broader or user-editable allowlist, add a `dangerous.ssrf_allowed_hosts` (or `network.allowed_hosts`) array to the config and merge it into the set passed to `ssrfGuardedTransport`.
393393

394+
### 18b. CGNAT and benchmark IP blocking
395+
396+
Go's `net.IP.IsPrivate()` covers RFC1918 and RFC4193 private ranges, but it does not cover RFC 6598 CGNAT (`100.64.0.0/10`) or the RFC 2544 benchmark-testing range (`198.18.0.0/15`). Tailscale and similar overlay networks use `100.64/10` addresses, so an attacker who could steer the agent to `http://100.x.x.x` (or a hostname that rebinding resolves to such an address) could reach unauthenticated internal services that the operator expected to be unreachable from the agent.
397+
398+
`internal/danger.IsBlockedIP` now blocks both ranges in addition to loopback, RFC1918, RFC4193, link-local, and unspecified addresses. Because this is the single source of truth used by `ClassifyURL` and the dial-time SSRF guard, the policy gate and the transport stay in sync.
399+
394400
### 19. MCP server environment sanitisation
395401

396402
MCP server subprocesses no longer inherit the full odek process environment. They receive only a minimal allowlist of safe variables (e.g. `PATH`, `HOME`, `LANG`, `TMPDIR`) plus any explicit `env` overrides from the server config. Keys matching secret patterns — `*_API_KEY`, `*_TOKEN`, `*_SECRET`, `*_PASSWORD`, `*_CREDENTIAL`, `*_PRIVATE_KEY`, etc. — are stripped even when listed in `env`. This prevents a compromised or malicious MCP server from reading secrets loaded from `~/.odek/secrets.env` or other provider keys that were present in the parent environment.

internal/danger/classifier.go

Lines changed: 30 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -330,20 +330,45 @@ func ClassifyURL(rawURL string) RiskClass {
330330
return NetworkEgress
331331
}
332332

333+
// extraBlockedNets contains IPv4 ranges that net.IP.IsPrivate does not cover
334+
// but which must still be unreachable to the agent's web tools:
335+
// - 100.64.0.0/10 RFC 6598 CGNAT (includes Tailscale)
336+
// - 198.18.0.0/15 RFC 2544 benchmark testing
337+
var extraBlockedNets []*net.IPNet
338+
339+
func init() {
340+
for _, cidr := range []string{"100.64.0.0/10", "198.18.0.0/15"} {
341+
_, n, err := net.ParseCIDR(cidr)
342+
if err != nil {
343+
panic(fmt.Sprintf("danger: invalid blocked CIDR %q: %v", cidr, err))
344+
}
345+
extraBlockedNets = append(extraBlockedNets, n)
346+
}
347+
}
348+
333349
// IsBlockedIP reports whether ip falls in a range that the agent's web tools
334350
// must never reach: loopback (127/8, ::1), RFC1918 / RFC4193 private (incl.
335351
// IPv6 ULA fc00::/7), link-local (169.254/16 — which covers the
336-
// 169.254.169.254 cloud-metadata endpoint — and fe80::/10), or the unspecified
352+
// 169.254.169.254 cloud-metadata endpoint — and fe80::/10), RFC 6598 CGNAT
353+
// (100.64/10), RFC 2544 benchmark testing (198.18/15), or the unspecified
337354
// address (0.0.0.0, ::). It is the single source of truth shared by both
338355
// ClassifyURL's literal-host gate and the dial-time SSRF guard, so the two
339356
// cannot drift apart. A nil IP is treated as blocked (fail closed).
340357
func IsBlockedIP(ip net.IP) bool {
341358
if ip == nil {
342359
return true
343360
}
344-
return ip.IsLoopback() || ip.IsPrivate() ||
361+
if ip.IsLoopback() || ip.IsPrivate() ||
345362
ip.IsLinkLocalUnicast() || ip.IsLinkLocalMulticast() ||
346-
ip.IsUnspecified()
363+
ip.IsUnspecified() {
364+
return true
365+
}
366+
for _, n := range extraBlockedNets {
367+
if n.Contains(ip) {
368+
return true
369+
}
370+
}
371+
return false
347372
}
348373

349374
// hostnameIsInternal reports whether a non-IP hostname denotes a well-known
@@ -2196,8 +2221,8 @@ func isNetworkEgress(first string, tokens []string) bool {
21962221
// gitCodeExecConfigKeys are git config keys whose values cause arbitrary
21972222
// command execution when set via -c/--config-env or git config.
21982223
var gitCodeExecConfigKeys = map[string]bool{
2199-
"core.pager": true,
2200-
"core.fsmonitor": true,
2224+
"core.pager": true,
2225+
"core.fsmonitor": true,
22012226
"credential.helper": true,
22022227
}
22032228

internal/danger/classifier_test.go

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1499,9 +1499,12 @@ func TestIsBlockedIP(t *testing.T) {
14991499
{"1.1.1.1", false},
15001500
{"93.184.216.34", false},
15011501
{"2606:4700:4700::1111", false},
1502-
// CGNAT 100.64/10 is not covered by Go's IsPrivate — documents the
1503-
// current (allowed) behavior so a future tightening is a conscious change.
1504-
{"100.64.0.1", false},
1502+
// CGNAT 100.64/10 (RFC 6598) and RFC 2544 benchmark testing
1503+
// 198.18/15 are not covered by Go's IsPrivate.
1504+
{"100.64.0.1", true},
1505+
{"100.127.255.255", true},
1506+
{"198.18.0.1", true},
1507+
{"198.19.255.255", true},
15051508
}
15061509
for _, tt := range tests {
15071510
t.Run(tt.ip, func(t *testing.T) {

0 commit comments

Comments
 (0)