From 97f1c89243a9c886d22fa1aa8fa26b2a15f01ee8 Mon Sep 17 00:00:00 2001 From: sneak Date: Tue, 20 Oct 2026 13:42:07 +0000 Subject: [PATCH] A path holding a character Go escapes reaches the app as the client sent it (closes #126) ReverseProxy sends the path URL.EscapedPath gives, which decodes and escapes again a path holding a character such as |, turning %2F into /. The path as sent now goes in URL.Opaque, which Go sends as it is; the rate limit and WAF exempt paths and the log line's path use the same path. A path starting with // is left as before: Go would send it in Opaque as a scheme and host. Model: opus-5-5 --- README.md | 13 ++++++---- internal/proxy/coreruleset.go | 2 +- internal/proxy/passthrough_test.go | 41 ++++++++++++++++++++++++++++++ internal/proxy/ratelimits_test.go | 3 +++ internal/proxy/request.go | 26 +++++++++++++------ 5 files changed, 71 insertions(+), 14 deletions(-) diff --git a/README.md b/README.md index ee4ad25..ca16173 100644 --- a/README.md +++ b/README.md @@ -96,11 +96,14 @@ in `bin/state` unless `SWWAF_STATE_DIR` is set, and the default rule file of ## What it does so far - Passes each request to the app and the app's answer back unchanged: method, - path, query, headers, body and status. Bodies stream through in both - directions and are never held whole in memory, but for the part of a request - body the Core Rule Set reads, at most `SWWAF_WAF_BODY_LIMIT` and one byte - more, which is held until the app is sent it. A WebSocket, or any other - upgraded connection, passes through, and the timeouts do not cut it. + path, query, headers, body and status. The one exception is a path starting + with `//` that holds a character Go escapes, such as `|`: Go cannot send it as + it is, and the app receives it decoded and escaped again, `%2F` as `/`. Bodies + stream through in both directions and are never held whole in memory, but for + the part of a request body the Core Rule Set reads, at most + `SWWAF_WAF_BODY_LIMIT` and one byte more, which is held until the app is sent + it. A WebSocket, or any other upgraded connection, passes through, and the + timeouts do not cut it. - Works out the client's address. A TCP peer outside `SWWAF_TRUSTED_PROXIES` is the client, and the forwarded headers it sends are replaced, not passed on. For a peer inside it, `X-Forwarded-For` is read from the right, and the first diff --git a/internal/proxy/coreruleset.go b/internal/proxy/coreruleset.go index 0f79a8e..223c9f3 100644 --- a/internal/proxy/coreruleset.go +++ b/internal/proxy/coreruleset.go @@ -23,7 +23,7 @@ import ( // limit while the Core Rule Set reads it, which it notes nothing of. func (rq *request) checkCoreRuleSet() string { cfg := rq.h.config - if cfg.WAFMode == config.WAFModeOff || pathExempt(rq.in.URL, cfg.WAFExemptPaths) { + if cfg.WAFMode == config.WAFModeOff || pathExempt(rq.in, cfg.WAFExemptPaths) { return "" } diff --git a/internal/proxy/passthrough_test.go b/internal/proxy/passthrough_test.go index 46cb72d..1b34821 100644 --- a/internal/proxy/passthrough_test.go +++ b/internal/proxy/passthrough_test.go @@ -74,6 +74,47 @@ func TestPassesRequestAndAnswerUnchanged(t *testing.T) { wantRequestFields(t, line, addr, len(requestBody), len(answerBody)) } +func TestPassesThePathAsSent(t *testing.T) { + t.Parallel() + + for _, path := range []string{ + // An encoded slash and a character Go escapes: |, ^, and é sent + // as its UTF-8 bytes. + "/a%2Fb|c", "/feat%2Fx/file^1", "/x%2Fy/café", + // A path starting with //, which Go would send as a scheme and + // host if it were put in URL.Opaque. + "//a%2Fb", + } { + t.Run(path, func(t *testing.T) { + t.Parallel() + + saw := make(chan string, 1) + app := startApp(t, func(_ http.ResponseWriter, r *http.Request) { + saw <- r.RequestURI + }) + addr, out := startProxy(t, app.URL, nil) + + conn := dial(t, addr) + send(t, conn, "GET "+path+"?q=1 HTTP/1.1\r\nHost: "+appHost+"\r\n\r\n") + + got := readResponse(t, conn) + if got.status != http.StatusOK { + t.Fatalf("status %d, want %d", got.status, http.StatusOK) + } + + target := <-saw + if target != path+"?q=1" { + t.Errorf("app saw %s, want %s", target, path+"?q=1") + } + + line := out.requestLine(t) + if line.Path != path { + t.Errorf("log line has path %s, want %s", line.Path, path) + } + }) + } +} + // wantAppSaw checks that the app received the test's request unchanged. func wantAppSaw(t *testing.T, saw appSaw, body []byte) { t.Helper() diff --git a/internal/proxy/ratelimits_test.go b/internal/proxy/ratelimits_test.go index 5d7408e..1b55ffc 100644 --- a/internal/proxy/ratelimits_test.go +++ b/internal/proxy/ratelimits_test.go @@ -185,6 +185,9 @@ func TestRateLimitCountsPathsThatAreNotExempt(t *testing.T) { "/assets/x%2Fy", "/assets/x%2fy", `/assets/x\y`, + // The same with a character Go escapes, |, for which Go's + // URL.EscapedPath gives /assets/x/y%7Cz, with no encoded slash. + "/assets/x%2Fy|z", } { t.Run(sent, func(t *testing.T) { t.Parallel() diff --git a/internal/proxy/request.go b/internal/proxy/request.go index e67599c..e4e09af 100644 --- a/internal/proxy/request.go +++ b/internal/proxy/request.go @@ -8,7 +8,6 @@ import ( "net/http/httptrace" "net/http/httputil" "net/netip" - "net/url" "os" "slices" "strings" @@ -22,6 +21,7 @@ import ( "sneak.berlin/go/smallwebwaf/internal/lookup" "sneak.berlin/go/smallwebwaf/internal/ratelimit" "sneak.berlin/go/smallwebwaf/internal/requestlog" + "sneak.berlin/go/smallwebwaf/internal/rules" ) // flushAfterEachWrite has ReverseProxy pass on each part of the app's @@ -137,7 +137,7 @@ func (h *handler) newRequest(w http.ResponseWriter, r *http.Request) *request { Method: r.Method, Scheme: scheme(r, peerTrusted), Host: r.Host, - Path: r.URL.EscapedPath(), + Path: rules.Path(r), Query: r.URL.RawQuery, Protocol: r.Proto, Referer: r.Referer(), @@ -284,7 +284,7 @@ func (rq *request) checkClient(ctx context.Context) string { } rq.counted = !isInside(rq.client, cfg.RateLimitExemptNets) && - !pathExempt(rq.in.URL, cfg.RateLimitExemptPaths) + !pathExempt(rq.in, cfg.RateLimitExemptPaths) if rq.counted { rq.limitPercent, rq.bytesPercent = rq.limitPercentages() rq.line.LimitPercent, rq.line.LimitPercentSetting = rq.limitPercent.logged() @@ -307,7 +307,7 @@ func (rq *request) checkClient(ctx context.Context) string { return rq.checkCoreRuleSet() } -// pathExempt reports whether a request for u is exempt under prefixes, +// pathExempt reports whether r is exempt under prefixes, // SWWAF_RATE_LIMIT_EXEMPT_PATHS or SWWAF_WAF_EXEMPT_PATHS: whether its // path as sent, the path the app receives, not percent-decoded, starts // with one of prefixes, so that /%61ssets/x is not under /assets/ for an @@ -316,10 +316,9 @@ func (rq *request) checkClient(ctx context.Context) string { // encoded slash (%2F or %2f), never is, since an app may act on it as a // path outside every prefix: /assets/..%2Flogin as /login, or /assets%2Fx // as one path segment, as Go's router does. -func pathExempt(u *url.URL, prefixes []string) bool { - decoded := u.Path - // EscapedPath is the path as the app receives it, not decoded. - sent := u.EscapedPath() +func pathExempt(r *http.Request, prefixes []string) bool { + decoded := r.URL.Path + sent := rules.Path(r) if strings.Contains(decoded, "..") || strings.Contains(decoded, `\`) || strings.Contains(strings.ToLower(sent), "%2f") { @@ -375,6 +374,17 @@ func (rq *request) rewrite(pr *httputil.ProxyRequest) { // ReverseProxy drops query parameters it cannot parse; the app gets // the query as the client sent it. pr.Out.URL.RawQuery = pr.In.URL.RawQuery + // ReverseProxy sends the path URL.EscapedPath gives, which, for a path + // holding a character Go escapes, such as |, is the whole path decoded + // and escaped again, %2F turned into /. Opaque is sent as it is, so the + // app gets the path as the client sent it. Left out are a target that + // is no path, such as *, and a path starting with //, which Go would + // send in Opaque as a scheme and host. + path := rules.Path(pr.In) + if strings.HasPrefix(path, "/") && !strings.HasPrefix(path, "//") { + pr.Out.URL.Opaque = path + } + setForwardedHeaders(pr.In, pr.Out, rq.peer, rq.peerTrusted) pr.Out.Header.Set(requestIDHeader, rq.line.RequestID) pr.Out.Header.Del(asnHeader)