Refuse dot segments in package paths and upstream auth matching

internal/config/config.go

@@ -1068,6 +1068,12 @@ func parseAuthURL(value string) (*url.URL, error) {
 }
 
 func authURLMatches(configured, target *url.URL) bool {
+	// An upstream resolves dot segments, so /scope/../other would leave the
+	// configured path while still matching it by prefix. Real package URLs
+	// never contain them, so they get no credentials at all.
+	if HasDotSegment(target.Path) {
+		return false
+	}
 	if !strings.EqualFold(configured.Scheme, target.Scheme) ||
 		!strings.EqualFold(configured.Hostname(), target.Hostname()) ||
 		authURLPort(configured) != authURLPort(target) {
@@ -1085,6 +1091,25 @@ func authURLMatches(configured, target *url.URL) bool {
 	return targetPath == configuredPath || strings.HasPrefix(targetPath, configuredPath+"/")
 }
 
+// HasDotSegment reports whether the URL path p, or p decoded once more, has a
+// '.' or '..' segment. Backslashes count as separators, and ";params" are
+// ignored, since some upstream servers (Tomcat, Jetty) treat them that way.
+func HasDotSegment(p string) bool {
+	candidates := []string{p}
+	if decoded, err := url.PathUnescape(p); err == nil {
+		candidates = append(candidates, decoded)
+	}
+	for _, candidate := range candidates {
+		for segment := range strings.SplitSeq(strings.ReplaceAll(candidate, `\`, "/"), "/") {
+			segment, _, _ = strings.Cut(segment, ";")
+			if segment == "." || segment == ".." {
+				return true
+			}
+		}
+	}
+	return false
+}
+
 func authURLPort(value *url.URL) string {
 	if port := value.Port(); port != "" {
 		return port

internal/config/config_test.go

@@ -1322,6 +1322,13 @@ func TestUpstreamAuthForURLMatchesURLComponents(t *testing.T) {
 		{name: "lookalike host rejected", url: "https://registry.example.com.evil.test/package"},
 		{name: "different scheme rejected", url: "http://registry.example.com/package"},
 		{name: "different port rejected", url: "https://registry.example.com:8443/package"},
+		{name: "dot dot segment rejected", url: "https://registry.example.com/private/../package"},
+		{name: "encoded dot dot segment rejected", url: "https://registry.example.com/private/%2e%2e/package"},
+		{name: "double encoded dot dot segment rejected", url: "https://registry.example.com/private/%252e%252e/package"},
+		{name: "dot segment rejected", url: "https://registry.example.com/private/./package"},
+		{name: "dot dot segment with parameter rejected", url: "https://registry.example.com/private/..;/package"},
+		{name: "encoded dot dot segment with parameter rejected", url: "https://registry.example.com/private/%2e%2e;x/package"},
+		{name: "dot dot segment with encoded parameter rejected", url: "https://registry.example.com/private/..%3b/package"},
 	}
 
 	for _, tt := range tests {

internal/server/dot_segments_test.go

@@ -0,0 +1,97 @@
+package server
+
+import (
+	"context"
+	"io"
+	"log/slog"
+	"net/http"
+	"net/http/httptest"
+	"testing"
+
+	"github.com/git-pkgs/proxy/internal/config"
+	"github.com/git-pkgs/proxy/internal/database"
+	"github.com/git-pkgs/proxy/internal/handler"
+	"github.com/git-pkgs/proxy/internal/storage"
+	"github.com/git-pkgs/registries/fetch"
+	"github.com/go-chi/chi/v5"
+)
+
+// TestProtocolHandlersRejectDotSegments sends paths with encoded '.' and '..'
+// segments, and with ';' path parameters, to every mounted ecosystem. Several
+// handlers append the decoded path to the upstream URL, where a '..' leaves
+// the configured upstream path, and Tomcat and Jetty drop ";params" first.
+func TestProtocolHandlersRejectDotSegments(t *testing.T) {
+	db, err := database.Create(t.TempDir() + "/test.db")
+	if err != nil {
+		t.Fatalf("creating database: %v", err)
+	}
+	t.Cleanup(func() { _ = db.Close() })
+	store, err := storage.OpenBucket(context.Background(), "file://"+t.TempDir())
+	if err != nil {
+		t.Fatalf("opening storage: %v", err)
+	}
+
+	transport := &countingTransport{}
+	client := &http.Client{Transport: transport}
+	fetcher := fetch.NewFetcher(fetch.WithHTTPClient(client), fetch.WithMaxRetries(0))
+	t.Cleanup(func() { _ = fetcher.Close() })
+	proxy := handler.NewProxy(db, store, fetcher, fetch.NewResolver(), slog.New(slog.NewTextHandler(io.Discard, nil)))
+	proxy.HTTPClient = client
+
+	cfg := config.Default()
+	cfg.BaseURL = "http://proxy.example"
+	s := &Server{cfg: cfg}
+	r := chi.NewRouter()
+	s.mountProtocolHandlers(r, proxy)
+
+	for _, target := range []string{
+		"/maven/%2e%2e/%2e%2e/secret",
+		"/maven/com/%2e%2e/%2e%2e/x/maven-metadata.xml",
+		"/maven/com/%2E%2E/x/1.0/x-1.0.jar",
+		"/maven/com/%2e/x/1.0/x-1.0.jar",
+		"/maven/com/%252e%252e/x/maven-metadata.xml",
+		"/conda/c/%2e%2e/x.json",
+		"/cran/src/contrib/%2e%2e%2fx.tar.gz",
+		"/cran/src/contrib/..%5cx.tar.gz",
+		"/go/a/%2e%2e/%2e%2e/x/@v/list",
+		"/gem/info/%2e%2e%2f%2e%2e%2fx",
+		"/gem/gems/%2e%2e%2fx-1.0.gem",
+		"/nuget/v3-flatcontainer/%2e%2e/x/index.json",
+		"/debian/pool/%2e%2e/x.deb",
+		"/rpm/%2e%2e/x.rpm",
+		"/maven/..;/..;/other-repo/com/x/x/1.0/x-1.0.jar",
+		"/maven/com/%2e%2e;/x/1.0/x-1.0.jar",
+		"/maven/com/..%3b/x/1.0/x-1.0.jar",
+		"/maven/com/evil/evil/1.0;x/evil-1.0.jar",
+		"/generic/github/o/r/releases/download/v1/tool.tar.gz;x",
+	} {
+		w := httptest.NewRecorder()
+		r.ServeHTTP(w, httptest.NewRequest(http.MethodGet, target, nil))
+		if w.Code != http.StatusBadRequest {
+			t.Errorf("%s: status = %d, want 400", target, w.Code)
+		}
+	}
+	if n := transport.hits.Load(); n != 0 {
+		t.Errorf("upstream was contacted %d times", n)
+	}
+}
+
+// TestDotSegmentGuardKeepsDottedNames keeps names that only contain dots,
+// such as versions and file extensions, working.
+func TestDotSegmentGuardKeepsDottedNames(t *testing.T) {
+	for _, target := range []string{
+		"/maven/com/example/x/1.0/x-1.0.jar",
+		"/pypi/simple/zope.interface/",
+		"/npm/.bin",
+		"/generic/github/o/r/releases/download/v1/...tar.gz",
+		"/maven/com/example/x/1..0/x-1..0.jar",
+	} {
+		var reached bool
+		next := http.HandlerFunc(func(http.ResponseWriter, *http.Request) { reached = true })
+		w := httptest.NewRecorder()
+		rejectDotSegments(next).ServeHTTP(w, httptest.NewRequest(http.MethodGet, target, nil))
+		if !reached {
+			t.Errorf("%s: status = %d, want the request passed on", target, w.Code)
+		}
+	}
+}

internal/server/middleware.go

@@ -8,6 +8,7 @@ import (
 	"time"
 
 	"github.com/git-pkgs/proxy/internal/accesslog"
+	"github.com/git-pkgs/proxy/internal/config"
 	"github.com/git-pkgs/proxy/internal/metrics"
 	"github.com/go-chi/chi/v5/middleware"
 )
@@ -124,15 +125,33 @@ func requestEcosystem(path string) string {
 	}
 }
 
-// rejectQueryInPath refuses requests whose decoded path contains '?' or '#'.
-// Handlers build upstream URLs from the decoded path, where such a character
-// would end the path: busybox.apk%3F/foo-9.9-r0.apk fetches busybox but is
-// parsed and cached as foo, under the wrong PURL and past the denylist.
-// Composer archives are exempt: their upstream URL comes from the package
-// metadata, and git branch versions such as dev-fix/#42 may contain '#'.
+// rejectQueryInPath refuses requests whose decoded path contains '?', '#'
+// or ';'. Handlers build upstream URLs from the decoded path, where '?' or
+// '#' would end the path: busybox.apk%3F/foo-9.9-r0.apk fetches busybox but
+// is parsed and cached as foo, under the wrong PURL and past the denylist.
+// Tomcat and Jetty drop ";params" from a path segment, so 1.0;x/evil-1.0.jar
+// would be served as version 1.0 while the proxy checks 1.0;x. Composer
+// archives are exempt: their upstream URL comes from the package metadata,
+// and git branch versions such as dev-fix/#42 may contain '#' or ';'.
 func rejectQueryInPath(next http.Handler) http.Handler {
 	return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
-		if strings.ContainsAny(r.URL.Path, "?#") && !strings.HasPrefix(r.URL.Path, "/composer/files/") {
+		if strings.ContainsAny(r.URL.Path, "?#;") && !strings.HasPrefix(r.URL.Path, "/composer/files/") {
+			http.Error(w, "invalid path", http.StatusBadRequest)
+			return
+		}
+		next.ServeHTTP(w, r)
+	})
+}
+
+// rejectDotSegments refuses requests whose decoded path has a '.' or '..'
+// segment. Routers clean literal dot segments, but encoded ones such as
+// %2e%2e reach the handlers, and several of them append the path to the
+// upstream URL, where the upstream resolves it outside the configured base
+// path. Backslashes count as separators, and the path is decoded once more
+// so double-encoded segments are refused too.
+func rejectDotSegments(next http.Handler) http.Handler {
+	return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		if config.HasDotSegment(r.URL.Path) {
 			http.Error(w, "invalid path", http.StatusBadRequest)
 			return
 		}

internal/server/query_in_path_test.go

@@ -93,15 +93,21 @@ func TestProtocolHandlersRejectQueryInPath(t *testing.T) {
 }
 
 // TestComposerFilesAllowHashInVersion keeps Composer archive downloads of git
-// branches with '#' in their name working: the version is escaped in the dist
-// URL and the archive is resolved from metadata, not from the request path.
+// branches with '#' or ';' in their name working: the version is escaped in
+// the dist URL and the archive is resolved from metadata, not from the
+// request path.
 func TestComposerFilesAllowHashInVersion(t *testing.T) {
-	var reached atomic.Bool
-	next := http.HandlerFunc(func(http.ResponseWriter, *http.Request) { reached.Store(true) })
+	for _, target := range []string{
+		"/composer/files/acme/lib/dev-fix%2F%2342/abc123.zip",
+		"/composer/files/acme/lib/dev-fix%3B42/abc123.zip",
+	} {
+		var reached atomic.Bool
+		next := http.HandlerFunc(func(http.ResponseWriter, *http.Request) { reached.Store(true) })
 
-	w := httptest.NewRecorder()
-	rejectQueryInPath(next).ServeHTTP(w, httptest.NewRequest(http.MethodGet, "/composer/files/acme/lib/dev-fix%2F%2342/abc123.zip", nil))
-	if w.Code == http.StatusBadRequest || !reached.Load() {
-		t.Errorf("status = %d, reached = %v; want the request passed on", w.Code, reached.Load())
+		w := httptest.NewRecorder()
+		rejectQueryInPath(next).ServeHTTP(w, httptest.NewRequest(http.MethodGet, target, nil))
+		if w.Code == http.StatusBadRequest || !reached.Load() {
+			t.Errorf("%s: status = %d, reached = %v; want the request passed on", target, w.Code, reached.Load())
+		}
 	}
 }

internal/server/server.go

@@ -493,7 +493,7 @@ func (s *Server) mountProtocolHandlers(r chi.Router, proxy *handler.Proxy) {
 	rpmHandler := handler.NewRPMHandlerWithUpstream(proxy, s.cfg.BaseURL, s.cfg.Upstream.RPM)
 	genericHandler := handler.NewGenericHandler(proxy, s.cfg.Upstream.Generic, s.cfg.Upstream.GenericArtifacts)
 
-	r = r.With(sandboxUpstreamContent, rejectQueryInPath)
+	r = r.With(sandboxUpstreamContent, rejectQueryInPath, rejectDotSegments)
 	r.Mount("/npm", http.StripPrefix("/npm", npmHandler.Routes()))
 	r.Mount("/cargo", http.StripPrefix("/cargo", cargoHandler.Routes()))
 	r.Mount("/gem", http.StripPrefix("/gem", gemHandler.Routes()))