rewrite: prevent placeholder re-expansion in injected query (#7761)

When the rewrite URI template ends with a literal '?' and contains a placeholder that expands to client-controlled bytes (e.g. {http.request.header.X-Fwd}), those bytes flow into buildQueryString which runs a second Replacer pass. If the bytes contain placeholder syntax such as {env.SECRET}, that placeholder is evaluated, allowing disclosure of environment variables, files (via {file./path}), or internal request vars through the rewritten request URI.

Escape '{' and '}' in the injected query before assigning it to the query variable, so the second pass cannot find any placeholder syntax to evaluate. Operator-written placeholders in the rewrite template are already expanded by the first pass on the path component, so the only '{' or '}' surviving into the injected query must have come from replacement values.

Fixes GHSA-j8px-rmrx-76h9.

Includes three regression tests mirroring the 'is not re-expanded' tests in modules/caddyhttp/vars_test.go.

Co-authored-by: Matt Holt <mholt@users.noreply.github.com>

modules/caddyhttp/rewrite/rewrite.go

@@ -223,6 +223,15 @@ func (rewr Rewrite) Rewrite(r *http.Request, repl *caddy.Replacer) bool {
 			newPath, injectedQuery = before, after
 			// don't overwrite explicitly-configured query string
 			if query == "" {
+				// the injected query came from the first-pass placeholder
+				// expansion above, which means any '{' or '}' bytes in it
+				// must have come from replacement values (e.g. a request
+				// header), not from operator-written placeholder syntax.
+				// escape them so buildQueryString does not re-expand them,
+				// which would allow attacker input like {env.SECRET} to be
+				// evaluated (see GHSA-j8px-rmrx-76h9).
+				injectedQuery = strings.ReplaceAll(injectedQuery, "{", "%7B")
+				injectedQuery = strings.ReplaceAll(injectedQuery, "}", "%7D")
 				query = injectedQuery
 			}
 		}

modules/caddyhttp/rewrite/rewrite_test.go

@@ -18,6 +18,7 @@ import (
 	"net/http"
 	"reflect"
 	"regexp"
+	"strings"
 	"testing"
 
 	"github.com/caddyserver/caddy/v2"
@@ -350,6 +351,32 @@ func TestRewrite(t *testing.T) {
 			input:  newRequest(t, "GET", "/foo//bar///baz?a=b//c"),
 			expect: newRequest(t, "GET", "/foo/bar/baz?a=b//c"),
 		},
+
+		// regression tests for GHSA-j8px-rmrx-76h9: when the rewrite URI
+		// ends with a literal '?', the first-pass placeholder expansion
+		// may produce a path containing attacker-controlled bytes that
+		// then get split at '?' and fed into buildQueryString, which runs
+		// a SECOND placeholder pass. Bytes injected via a header value (or
+		// any other client-controlled placeholder) must not be treated as
+		// placeholder syntax during this second pass.
+		{
+			// literal header value containing placeholder syntax is not re-expanded into query
+			rule:   Rewrite{URI: "/serve/{http.request.header.X-Fwd}?"},
+			input:  newRequestWithHeader(t, "GET", "/anything", "X-Fwd", "foo?{env.CADDY_REWRITE_TEST_SECRET}=leak"),
+			expect: newRequest(t, "GET", "/serve/foo?%7Benv.CADDY_REWRITE_TEST_SECRET%7D=leak"),
+		},
+		{
+			// literal header value with placeholder syntax in query position is not re-expanded
+			rule:   Rewrite{URI: "/serve/{http.request.header.X-Fwd}?"},
+			input:  newRequestWithHeader(t, "GET", "/anything", "X-Fwd", "ok?key={env.CADDY_REWRITE_TEST_SECRET}"),
+			expect: newRequest(t, "GET", "/serve/ok?key=%7Benv.CADDY_REWRITE_TEST_SECRET%7D"),
+		},
+		{
+			// literal header value with embedded file placeholder is not re-expanded
+			rule:   Rewrite{URI: "/serve/{http.request.header.X-Fwd}?"},
+			input:  newRequestWithHeader(t, "GET", "/anything", "X-Fwd", "ok?path={file./etc/passwd}"),
+			expect: newRequest(t, "GET", "/serve/ok?path=%7Bfile./etc/passwd%7D"),
+		},
 	} {
 		// copy the original input just enough so that we can
 		// compare it after the rewrite to see if it changed
@@ -364,6 +391,9 @@ func TestRewrite(t *testing.T) {
 		repl.Set("http.request.uri", tc.input.RequestURI)
 		repl.Set("http.request.uri.path", tc.input.URL.Path)
 		repl.Set("http.request.uri.query", tc.input.URL.RawQuery)
+		for field, vals := range tc.input.Header {
+			repl.Set("http.request.header."+field, strings.Join(vals, ","))
+		}
 
 		// we can't directly call Provision() without a valid caddy.Context
 		// (TODO: fix that) so here we ad-hoc compile the regex
@@ -456,6 +486,12 @@ func newRequest(t *testing.T, method, uri string) *http.Request {
 	return req
 }
 
+func newRequestWithHeader(t *testing.T, method, uri, headerKey, headerVal string) *http.Request {
+	req := newRequest(t, method, uri)
+	req.Header.Set(headerKey, headerVal)
+	return req
+}
+
 // reqEqual if r1 and r2 are equal enough for our purposes.
 func reqEqual(r1, r2 *http.Request) bool {
 	if r1.Method != r2.Method {