VULN-141859: Normalize request path when using access rules

With the previous changes made to normalize request paths, we faced some issues:

* Regressions in Next.js route groups: https://github.com/cloudflare/cloudflared/issues/1719
* WordPress trailing slashes: https://github.com/cloudflare/cloudflared/issues/1717

This PR attempts to fix a vulnerability regarding un-normalized request paths using a different approach:
instead of mutating `req.URL.Path` before forwarding to the origin, we compute a cleaned path used only for
ingress rule selection, leaving the original request completely untouched. This separates the security classification decision from request proxying.

Since this can be a breaking change, we are adding it disabled by default and will only enable it in future releases.

cmd/cloudflared/flags/flags.go

@@ -129,6 +129,9 @@ const (
 	// NoPrechecks is the command line flag to skip connectivity pre-checks at startup.
 	NoPrechecks = "no-prechecks"
 
+	// DisablePathNormalization disables URL path normalization before matching ingress rules.
+	DisablePathNormalization = "disable-path-normalization"
+
 	// LogLevel is the command line flag for the cloudflared logging level
 	LogLevel = "loglevel"
 

cmd/cloudflared/tunnel/cmd.go

@@ -984,6 +984,14 @@ func configureProxyFlags(shouldHide bool) []cli.Flag {
 			Value:   false,
 			Hidden:  shouldHide,
 		}),
+		altsrc.NewBoolFlag(&cli.BoolFlag{
+			Name: cfdflags.DisablePathNormalization,
+			Usage: "Disable URL path normalization before matching ingress rules. Use this setting at your own risk." +
+				"An unnormalized request path could bypass Cloudflare Access rules and reach a protected origin.",
+			EnvVars: []string{"TUNNEL_DISABLE_PATH_NORMALIZATION"},
+			Value:   true,
+			Hidden:  shouldHide,
+		}),
 		altsrc.NewDurationFlag(&cli.DurationFlag{
 			Name:   ingress.ProxyConnectTimeoutFlag,
 			Usage:  legacyTunnelFlag("HTTP proxy timeout for establishing a new connection"),

cmd/cloudflared/tunnel/configuration.go

@@ -264,10 +264,11 @@ func prepareTunnelConfig(
 		tunnelConfig.ICMPRouterServer = icmpRouter
 	}
 	orchestratorConfig := &orchestration.Config{
-		Ingress:             &ingressRules,
-		WarpRouting:         warpRoutingConfig,
-		OriginDialerService: originDialerService,
-		ConfigurationFlags:  parseConfigFlags(c),
+		Ingress:                  &ingressRules,
+		WarpRouting:              warpRoutingConfig,
+		OriginDialerService:      originDialerService,
+		DisablePathNormalization: c.Bool(flags.DisablePathNormalization),
+		ConfigurationFlags:       parseConfigFlags(c),
 	}
 	return tunnelConfig, orchestratorConfig, nil
 }

ingress/ingress.go

@@ -4,6 +4,7 @@ import (
 	"fmt"
 	"net"
 	"net/url"
+	"path"
 	"regexp"
 	"strconv"
 	"strings"
@@ -44,9 +45,14 @@ func (ing Ingress) FindMatchingRule(hostname, path string) (*Rule, int) {
 	if err == nil {
 		hostname = host
 	}
+	// Canonicalize before first-match evaluation so an earlier non-Access rule
+	// cannot capture a traversal targeting a protected rule.
+	if !ing.DisablePathNormalization && (rulesRequireAccess(ing.InternalRules) || rulesRequireAccess(ing.Rules)) {
+		path = canonicalPathForMatching(path)
+	}
 	for i, rule := range ing.InternalRules {
 		if rule.Matches(hostname, path) {
-			// Local rule matches return a negative rule index to distiguish local rules from user-defined rules in logs
+			// Local rule matches return a negative rule index to distinguish local rules from user-defined rules in logs
 			// Full range would be [-1 .. )
 			return &rule, -1 - i
 		}
@@ -61,6 +67,40 @@ func (ing Ingress) FindMatchingRule(hostname, path string) (*Rule, int) {
 	return &ing.Rules[i], i
 }
 
+func rulesRequireAccess(rules []Rule) bool {
+	for _, rule := range rules {
+		if rule.Config.Access.Required {
+			return true
+		}
+	}
+	return false
+}
+
+// canonicalPathForMatching resolves dot-segments in URL.Path for ingress rule
+// matching only; the request forwarded to the origin is never modified.
+// net/url decodes percent-encoded separators and dot-segments into URL.Path
+// before this is called. Backslashes are then converted to forward slashes to
+// match edge normalization before path.Clean resolves repeated separators and
+// dot-segments. Trailing slashes are preserved, including the RFC 3986 implied
+// slash after a trailing "." or ".." segment.
+func canonicalPathForMatching(p string) string {
+	if p == "" {
+		return "/"
+	}
+	p = strings.ReplaceAll(p, `\`, "/")
+	// In RFC 3986, a trailing "." or ".." segment implies a directory, so the
+	// cleaned result should end with "/". path.Clean removes the dot-segment
+	// but also strips the slash, so we detect it up front.
+	trailingSlash := strings.HasSuffix(p, "/") ||
+		strings.HasSuffix(p, "/.") ||
+		strings.HasSuffix(p, "/..")
+	cleaned := path.Clean(p)
+	if trailingSlash && cleaned != "/" {
+		cleaned += "/"
+	}
+	return cleaned
+}
+
 func matchHost(ruleHost, reqHost string) bool {
 	if ruleHost == reqHost {
 		return true
@@ -81,6 +121,9 @@ type Ingress struct {
 	// Rules that are provided by the user from remote or local configuration
 	Rules    []Rule              `json:"ingress"`
 	Defaults OriginRequestConfig `json:"originRequest"`
+	// DisablePathNormalization is a process-local override applied by the orchestrator.
+	// Enabling it restores the Access bypass described by VULN-141859.
+	DisablePathNormalization bool `json:"-" yaml:"-"`
 }
 
 // ParseIngress parses ingress rules, but does not send HTTP requests to the origins.

ingress/ingress_test.go

@@ -555,7 +555,7 @@ func TestSingleOriginServices(t *testing.T) {
 		flagSet.String("unix-socket", "", "")
 		cliCtx := cli.NewContext(cli.NewApp(), flagSet, nil)
 		for i := 0; i < len(params); i += 2 {
-			cliCtx.Set(params[i], params[i+1])
+			_ = cliCtx.Set(params[i], params[i+1])
 		}
 
 		return cliCtx
@@ -605,7 +605,7 @@ func TestSingleOriginServices(t *testing.T) {
 			if test.err != nil {
 				return
 			}
-			require.Equal(t, 1, len(ingress.Rules))
+			require.Len(t, ingress.Rules, 1)
 			rule := ingress.Rules[0]
 			require.Equal(t, test.expectedService, rule.Service)
 		})
@@ -626,7 +626,7 @@ func TestSingleOriginServices_URL(t *testing.T) {
 		flagSet := flag.NewFlagSet(t.Name(), flag.PanicOnError)
 		flagSet.String("url", "", "")
 		cliCtx := cli.NewContext(cli.NewApp(), flagSet, nil)
-		cliCtx.Set(param, value)
+		_ = cliCtx.Set(param, value)
 		return cliCtx
 	}
 
@@ -636,7 +636,7 @@ func TestSingleOriginServices_URL(t *testing.T) {
 			url := urlMustParse(test + host)
 			ingress, err := parseCLIIngress(newCli("url", url.String()), false)
 			require.NoError(t, err)
-			require.Equal(t, 1, len(ingress.Rules))
+			require.Len(t, ingress.Rules, 1)
 			rule := ingress.Rules[0]
 			require.Equal(t, &httpService{url: url}, rule.Service)
 		})
@@ -648,7 +648,7 @@ func TestSingleOriginServices_URL(t *testing.T) {
 			url := urlMustParse(test + host)
 			ingress, err := parseCLIIngress(newCli("url", url.String()), false)
 			require.NoError(t, err)
-			require.Equal(t, 1, len(ingress.Rules))
+			require.Len(t, ingress.Rules, 1)
 			rule := ingress.Rules[0]
 			require.Equal(t, newTCPOverWSService(url), rule.Service)
 		})
@@ -712,7 +712,7 @@ func TestFindMatchingRule(t *testing.T) {
 
 	for _, test := range tests {
 		_, ruleIndex := ingress.FindMatchingRule(test.host, test.path)
-		assert.Equal(t, test.wantRuleIndex, ruleIndex, fmt.Sprintf("Expect host=%s, path=%s to match rule %d, got %d", test.host, test.path, test.wantRuleIndex, ruleIndex))
+		assert.Equal(t, test.wantRuleIndex, ruleIndex, "Expect host=%s, path=%s to match rule %d, got %d", test.host, test.path, test.wantRuleIndex, ruleIndex)
 	}
 }
 
@@ -840,3 +840,213 @@ func MustReadIngress(s string) *config.Configuration {
 	}
 	return &conf
 }
+
+func TestCanonicalPathForMatching(t *testing.T) {
+	t.Parallel()
+
+	tests := []struct {
+		name  string
+		input string
+		want  string
+	}{
+		// Dot-segment traversal — the security-relevant cases.
+		{name: "literal dot-dot", input: "/public/../admin", want: "/admin"},
+		{name: "single dot collapsed", input: "/public/./admin", want: "/public/admin"},
+		{name: "multiple dot-dot segments", input: "/a/b/c/../../..", want: "/"},
+		{name: "deep traversal", input: "/a/b/c/../../../etc/passwd", want: "/etc/passwd"},
+		{name: "double slash collapsed", input: "//etc/passwd", want: "/etc/passwd"},
+		{name: "backslashes normalized", input: `\etc\passwd`, want: "/etc/passwd"},
+		{name: "repeated backslashes collapsed", input: `\\etc\\passwd`, want: "/etc/passwd"},
+		{name: "mixed separators collapsed", input: `/\etc//passwd`, want: "/etc/passwd"},
+		{name: "mixed dot and dot-dot", input: "/a/./b/../c", want: "/a/c"},
+
+		// Trailing slash preservation (including RFC 3986 §5.2.4 implied slash).
+		{name: "trailing slash preserved", input: "/contact/", want: "/contact/"},
+		{name: "trailing dot implies slash", input: "/admin/.", want: "/admin/"},
+		{name: "trailing dot-dot implies slash", input: "/admin/..", want: "/"},
+		{name: "trailing dot with slash", input: "/admin/./", want: "/admin/"},
+		{name: "trailing dot-dot with slash", input: "/admin/../", want: "/"},
+
+		// Normal paths — must be unchanged.
+		{name: "simple path", input: "/api/v1/users", want: "/api/v1/users"},
+		{name: "root", input: "/", want: "/"},
+		{name: "empty becomes root", input: "", want: "/"},
+		{name: "parentheses in segment", input: "/app/(auth)/page.js", want: "/app/(auth)/page.js"},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			t.Parallel()
+			assert.Equal(t, tt.want, canonicalPathForMatching(tt.input))
+		})
+	}
+}
+
+func TestCanonicalPathForMatchingEncodedSeparators(t *testing.T) {
+	t.Parallel()
+
+	tests := []struct {
+		name  string
+		input string
+	}{
+		{name: "backslash", input: `\`},
+		{name: "encoded backslash", input: "%5C"},
+		{name: "two encoded backslashes", input: "%5C%5C"},
+		{name: "encoded slash", input: "%2f"},
+		{name: "two encoded slashes", input: "%2f%2f"},
+		{name: "encoded and literal slashes", input: "%2f/"},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			t.Parallel()
+			urlPath, err := url.PathUnescape(tt.input)
+			require.NoError(t, err)
+			assert.Equal(t, "/", canonicalPathForMatching(urlPath))
+		})
+	}
+}
+
+// TestFindMatchingRuleTraversal verifies that path traversal cannot be used to
+// bypass an ingress rule's Access middleware by sending a request whose raw
+// path does not match the protected rule but whose canonical path resolves to
+// a protected resource.
+//
+// The non-Access /public rule deliberately precedes the Access-protected
+// /admin rule to verify that raw-path matching cannot short-circuit selection.
+func TestFindMatchingRuleTraversal(t *testing.T) {
+	t.Parallel()
+
+	const rulesYAML = `
+ingress:
+ - hostname: app.example.com
+   path: "^/public"
+   service: https://localhost:8080
+ - hostname: app.example.com
+   path: "^/admin"
+   service: https://localhost:8081
+   originRequest:
+     access:
+       required: true
+       teamName: team
+ - hostname: app.example.com
+   service: https://localhost:8082
+ - service: http_status:404
+`
+	ing, err := ParseIngress(MustReadIngress(rulesYAML))
+	require.NoError(t, err)
+
+	tests := []struct {
+		name        string
+		rawURL      string
+		wantRuleNum int // 0 = /public, 1 = /admin with Access, 2 = catch-all
+	}{
+		// Public requests must match the unprotected catch-all.
+		{
+			name:        "normal public path",
+			rawURL:      "https://app.example.com/public/index.html",
+			wantRuleNum: 0,
+		},
+		{
+			name:        "single dot inside public stays in public",
+			rawURL:      "https://app.example.com/public/./index.html",
+			wantRuleNum: 0,
+		},
+		{
+			name:        "normal admin path",
+			rawURL:      "https://app.example.com/admin/index.html",
+			wantRuleNum: 1,
+		},
+
+		// Traversal via literal dot-dot must match the protected rule.
+		{
+			name:        "literal dot-dot enters admin",
+			rawURL:      "https://app.example.com/public/../admin",
+			wantRuleNum: 1,
+		},
+		{
+			name:        "deep traversal enters admin",
+			rawURL:      "https://app.example.com/public/subdir/../../admin",
+			wantRuleNum: 1,
+		},
+
+		// Traversal via percent-encoded dot-segments.
+		// net/url decodes these into URL.Path before FindMatchingRule is called,
+		// so canonicalPathForMatching resolves them identically to literal dot-dot.
+		{
+			name:        "%2e%2e encoded dot-dot enters admin",
+			rawURL:      "https://app.example.com/public/%2e%2e/admin",
+			wantRuleNum: 1,
+		},
+		{
+			name:        "%2E%2E uppercase encoded dot-dot enters admin",
+			rawURL:      "https://app.example.com/public/%2E%2E/admin",
+			wantRuleNum: 1,
+		},
+		{
+			// ..%2f encodes the slash; net/url decodes it to "../" in URL.Path.
+			name:        "..%2f encoded slash enters admin",
+			rawURL:      "https://app.example.com/public/..%2fadmin",
+			wantRuleNum: 1,
+		},
+		{
+			// .%2e is another way to encode ".."; net/url decodes it to ".." in URL.Path.
+			name:        ".%2e mixed encoding enters admin",
+			rawURL:      "https://app.example.com/public/.%2e/admin",
+			wantRuleNum: 1,
+		},
+		{
+			name:        "encoded backslashes enter admin",
+			rawURL:      "https://app.example.com/public%5C..%5Cadmin",
+			wantRuleNum: 1,
+		},
+		{
+			name:        "repeated encoded and mixed separators enter admin",
+			rawURL:      "https://app.example.com/public%5C%5C..%2fadmin",
+			wantRuleNum: 1,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			t.Parallel()
+			u, err := url.Parse(tt.rawURL)
+			require.NoError(t, err)
+			_, ruleNum := ing.FindMatchingRule(u.Host, u.Path)
+			assert.Equal(t, tt.wantRuleNum, ruleNum)
+		})
+	}
+}
+
+func TestFindMatchingRuleDoesNotCanonicalizeWithoutAccess(t *testing.T) {
+	t.Parallel()
+
+	ing := Ingress{Rules: []Rule{
+		{Hostname: "app.example.com", Path: MustParsePath(t, "^/public")},
+		{Hostname: "app.example.com", Path: MustParsePath(t, "^/admin")},
+		{},
+	}}
+
+	_, ruleNum := ing.FindMatchingRule("app.example.com", "/public/../admin")
+	assert.Equal(t, 0, ruleNum)
+}
+
+func TestFindMatchingRulePathNormalizationCanBeDisabled(t *testing.T) {
+	t.Parallel()
+
+	ing := Ingress{
+		Rules: []Rule{
+			{Hostname: "app.example.com", Path: MustParsePath(t, "^/public")},
+			{
+				Hostname: "app.example.com",
+				Path:     MustParsePath(t, "^/admin"),
+				Config:   OriginRequestConfig{Access: config.AccessConfig{Required: true}},
+			},
+			{},
+		},
+		DisablePathNormalization: true,
+	}
+
+	_, ruleNum := ing.FindMatchingRule("app.example.com", "/public/../admin")
+	assert.Equal(t, 0, ruleNum)
+}

ingress/rule.go

@@ -43,7 +43,7 @@ func (r Rule) MultiLineString() string {
 	}
 	if r.Path != nil && r.Path.Regexp != nil {
 		out.WriteString("\tpath: ")
-		out.WriteString(r.Path.Regexp.String())
+		out.WriteString(r.Path.String())
 		out.WriteRune('\n')
 	}
 	out.WriteString("\tservice: ")
@@ -53,17 +53,12 @@ func (r Rule) MultiLineString() string {
 
 // Matches checks if the rule matches a given hostname/path combination.
 func (r *Rule) Matches(hostname, path string) bool {
-	hostMatch := false
-	if r.Hostname == "" || r.Hostname == "*" {
-		hostMatch = true
-	} else {
-		hostMatch = matchHost(r.Hostname, hostname)
-	}
+	hostMatch := r.Hostname == "" || r.Hostname == "*" || matchHost(r.Hostname, hostname)
 	punycodeHostMatch := false
 	if r.punycodeHostname != "" {
 		punycodeHostMatch = matchHost(r.punycodeHostname, hostname)
 	}
-	pathMatch := r.Path == nil || r.Path.Regexp == nil || r.Path.Regexp.MatchString(path)
+	pathMatch := r.Path == nil || r.Path.Regexp == nil || r.Path.MatchString(path)
 	return (hostMatch || punycodeHostMatch) && pathMatch
 }
 
@@ -76,5 +71,5 @@ func (r *Regexp) MarshalJSON() ([]byte, error) {
 	if r.Regexp == nil {
 		return json.Marshal(nil)
 	}
-	return json.Marshal(r.Regexp.String())
+	return json.Marshal(r.String())
 }

ingress/rule_test.go

@@ -119,7 +119,7 @@ func Test_rule_matches(t *testing.T) {
 			name: "Hostname and path",
 			rule: Rule{
 				Hostname: "*.example.com",
-				Path:     &Regexp{Regexp: regexp.MustCompile("/static/.*\\.html")},
+				Path:     &Regexp{Regexp: regexp.MustCompile(`/static/.*\.html`)},
 			},
 			args: args{
 				requestURL: MustParseURL(t, "https://www.example.com/static/index.html"),
@@ -158,6 +158,16 @@ func Test_rule_matches(t *testing.T) {
 			},
 			want: false,
 		},
+		{
+			name: "Rule matches original path",
+			rule: Rule{
+				Path: &Regexp{Regexp: regexp.MustCompile("^/admin")},
+			},
+			args: args{
+				requestURL: MustParseURL(t, "https://example.com/public/../admin"),
+			},
+			want: false,
+		},
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
@@ -186,6 +196,7 @@ func TestStaticHTTPStatus(t *testing.T) {
 		w := httptest.NewRecorder()
 		n, err := io.Copy(w, resp.Body)
 		require.NoError(t, err)
+		require.NoError(t, resp.Body.Close())
 		require.Equal(t, int64(0), n)
 	}
 	sendReq()

logger/create.go

@@ -190,7 +190,7 @@ func createConsoleLogger(config ConsoleConfig) io.Writer {
 	consoleOut := os.Stderr
 	return zerolog.ConsoleWriter{
 		Out:        colorable.NewColorable(consoleOut),
-		NoColor:    config.noColor || !term.IsTerminal(int(consoleOut.Fd())), // nolint:gosec
+		NoColor:    config.noColor || !term.IsTerminal(int(consoleOut.Fd())), //nolint:gosec // Fd() returns a uintptr that safely fits in int on all supported platforms
 		TimeFormat: consoleTimeFormat,
 	}
 }

orchestration/config.go

@@ -22,6 +22,8 @@ type Config struct {
 	Ingress             *ingress.Ingress
 	WarpRouting         ingress.WarpRoutingConfig
 	OriginDialerService *ingress.OriginDialerService
+	// DisablePathNormalization is a process-local override for ingress matching.
+	DisablePathNormalization bool
 
 	// Extra settings used to configure this instance but that are not eligible for remotely management
 	// ie. (--protocol, --loglevel, ...)

orchestration/orchestrator.go

@@ -187,6 +187,8 @@ func (o *Orchestrator) updateIngress(ingressRules ingress.Ingress, warpRouting i
 
 	// Assign the internal ingress rules to the parsed ingress
 	ingressRules.InternalRules = o.internalRules
+	// Path normalization is a process-local CLI setting and must survive remote updates.
+	ingressRules.DisablePathNormalization = o.config.DisablePathNormalization
 
 	// Check if ingress rules are empty, and add the default route if so.
 	if ingressRules.IsEmpty() {

orchestration/orchestrator_test.go

@@ -67,8 +67,9 @@ func TestUpdateConfiguration(t *testing.T) {
 		TCPWriteTimeout: 1 * time.Second,
 	}, &testLogger)
 	initConfig := &Config{
-		Ingress:             &ingress.Ingress{},
-		OriginDialerService: originDialer,
+		Ingress:                  &ingress.Ingress{},
+		OriginDialerService:      originDialer,
+		DisablePathNormalization: true,
 	}
 	orchestrator, err := NewOrchestrator(t.Context(), initConfig, testTags, []ingress.Rule{ingress.NewManagementRule(management.New("management.argotunnel.com", false, "1.1.1.1:80", uuid.Nil, "", &testLogger, nil))}, &testLogger)
 	require.NoError(t, err)
@@ -113,6 +114,7 @@ func TestUpdateConfiguration(t *testing.T) {
 
 	updateWithValidation(t, orchestrator, 2, configJSONV2)
 	configV2 := orchestrator.config
+	require.True(t, configV2.Ingress.DisablePathNormalization)
 	// Validate internal ingress rules
 	require.Equal(t, "management.argotunnel.com", configV2.Ingress.InternalRules[0].Hostname)
 	require.True(t, configV2.Ingress.InternalRules[0].Matches("management.argotunnel.com", "/ping"))