diff --git a/ipn/ipnlocal/serve.go b/ipn/ipnlocal/serve.go index 7a63fdbf0..8fdf66f4d 100644 --- a/ipn/ipnlocal/serve.go +++ b/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 + } } } diff --git a/ipn/ipnlocal/serve_test.go b/ipn/ipnlocal/serve_test.go index 8287d99a0..205dc674e 100644 --- a/ipn/ipnlocal/serve_test.go +++ b/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)