ipn/ipnlocal: update getServeHandler path handling on malformed url (#20431)

This commit updates the path matching logic in getServeHandler for malformed
request targets like "*" (e.g. "GET *") and "" (e.g. "CONNECT" authority-form).
Those paths never reduce to "/" as absolute path would. An absolute path check
was added and an additional check on no further reduce was added in the loop.

Fixes tailscale/corp#44814

Signed-off-by: kevinliang10 <kevinliang@tailscale.com>

ipn/ipnlocal/serve.go

@@ -818,6 +818,15 @@ func (b *LocalBackend) getServeHandler(r *http.Request) (_ ipn.HTTPHandlerView,
 		return h, r.URL.Path, true
 	}
 	pth := path.Clean(r.URL.Path)
+	// A well-formed origin-form request path is absolute. Malformed request
+	// targets — "*" (e.g. "GET *") and "" (e.g. "CONNECT" authority-form),
+	// clean to "*" and "." respectively. Those are path.Dir fixed points that
+	// never equal "/" and match no mount, so without this guard the loop below
+	// would spin forever on one CPU core (a remote DoS via serve, or via funnel
+	// from the internet).
+	if !strings.HasPrefix(pth, "/") {
+		return z, "", false
+	}
 	for {
 		withSlash := pth + "/"
 		if h, ok := wsc.Handlers().GetOk(withSlash); ok {
@@ -829,7 +838,13 @@ func (b *LocalBackend) getServeHandler(r *http.Request) (_ ipn.HTTPHandlerView,
 		if pth == "/" {
 			return z, "", false
 		}
-		pth = path.Dir(pth)
+		// Belt-and-suspenders with the absolute-path check above: stop if
+		// path.Dir stops shrinking rather than assuming it always reaches "/".
+		if parent := path.Dir(pth); parent != pth {
+			pth = parent
+		} else {
+			return z, "", false
+		}
 	}
 }
 

ipn/ipnlocal/serve_test.go

@@ -190,6 +190,34 @@ func TestGetServeHandler(t *testing.T) {
 			path: "/foo/../../../../../../../../etc/passwd",
 			want: "/",
 		},
+		// Malformed request targets that net/http hands the handler verbatim.
+		// These clean to a path.Dir fixed point ("*" or ".") that never reaches
+		// "/", and once spun the getServeHandler loop below forever (a remote
+		// serve/funnel DoS). They must resolve to not-found, not hang.
+		{
+			name: "asterisk", // "GET *"
+			conf: conf1,
+			path: "*",
+			want: "",
+		},
+		{
+			name: "empty", // "CONNECT" authority-form sets URL.Path to ""
+			conf: conf1,
+			path: "",
+			want: "",
+		},
+		{
+			name: "dot",
+			conf: conf1,
+			path: ".",
+			want: "",
+		},
+		{
+			name: "asterisk-subpath",
+			conf: conf1,
+			path: "*/foo",
+			want: "",
+		},
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
@@ -209,6 +237,9 @@ func TestGetServeHandler(t *testing.T) {
 				DestPort: port,
 			}))
 
+			// A malformed target like "*" or "" once spun getServeHandler's
+			// path-walk loop forever; a regression would hang here until the
+			// package test timeout fires.
 			h, got, ok := b.getServeHandler(req)
 			if (got != "") != ok {
 				t.Fatalf("got ok=%v, but got mountPoint=%q", ok, got)