From f3840d56af3ffe96d65d3fdc03d779a762d3afe2 Mon Sep 17 00:00:00 2001
From: yusing <yusing@6uo.me>
Date: Sat, 22 Mar 2025 23:53:33 +0800
Subject: [PATCH] security: sanitize path and uri

---
 internal/api/v1/favicon/favicon.go  | 13 ++----
 internal/api/v1/new_agent.go        | 10 ++++-
 internal/utils/strutils/filepath.go | 11 +++++
 internal/utils/strutils/url.go      | 20 +++++++++
 internal/utils/strutils/url_test.go | 63 +++++++++++++++++++++++++++++
 5 files changed, 106 insertions(+), 11 deletions(-)
 create mode 100644 internal/utils/strutils/filepath.go
 create mode 100644 internal/utils/strutils/url.go
 create mode 100644 internal/utils/strutils/url_test.go

diff --git a/internal/api/v1/favicon/favicon.go b/internal/api/v1/favicon/favicon.go
index cdd8455..ccbb9a3 100644
--- a/internal/api/v1/favicon/favicon.go
+++ b/internal/api/v1/favicon/favicon.go
@@ -19,6 +19,7 @@ import (
 	gphttp "github.com/yusing/go-proxy/internal/net/gphttp"
 	"github.com/yusing/go-proxy/internal/route/routes"
 	route "github.com/yusing/go-proxy/internal/route/types"
+	"github.com/yusing/go-proxy/internal/utils/strutils"
 )
 
 type fetchResult struct {
@@ -207,10 +208,7 @@ func findIconSlow(r route.HTTPRoute, req *http.Request, uri string) *fetchResult
 	defer cancel()
 	newReq := req.WithContext(ctx)
 	newReq.Header.Set("Accept-Encoding", "identity") // disable compression
-	if !strings.HasPrefix(uri, "/") {
-		uri = "/" + uri
-	}
-	u, err := url.ParseRequestURI(uri)
+	u, err := url.ParseRequestURI(strutils.SanitizeURI(uri))
 	if err != nil {
 		logging.Error().Err(err).
 			Str("route", r.TargetName()).
@@ -231,11 +229,8 @@ func findIconSlow(r route.HTTPRoute, req *http.Request, uri string) *fetchResult
 			return &fetchResult{statusCode: http.StatusBadGateway, errMsg: "connection error"}
 		default:
 			if loc := c.Header().Get("Location"); loc != "" {
-				loc = path.Clean(loc)
-				if !strings.HasPrefix(loc, "/") {
-					loc = "/" + loc
-				}
-				if loc == newReq.URL.Path {
+				loc = strutils.SanitizeURI(loc)
+				if loc == "/" || loc == newReq.URL.Path {
 					return &fetchResult{statusCode: http.StatusBadGateway, errMsg: "circular redirect"}
 				}
 				return findIconSlow(r, req, loc)
diff --git a/internal/api/v1/new_agent.go b/internal/api/v1/new_agent.go
index 2f1a124..7c381f1 100644
--- a/internal/api/v1/new_agent.go
+++ b/internal/api/v1/new_agent.go
@@ -126,11 +126,17 @@ func VerifyNewAgent(w http.ResponseWriter, r *http.Request) {
 		return
 	}
 
-	if err := os.WriteFile(certs.AgentCertsFilename(data.Host), zip, 0600); err != nil {
+	filename := certs.AgentCertsFilename(data.Host)
+	if !strutils.IsValidFilename(filename) {
+		gphttp.ClientError(w, gphttp.ErrInvalidKey("host"))
+		return
+	}
+
+	if err := os.WriteFile(filename, zip, 0600); err != nil {
 		gphttp.ServerError(w, r, err)
 		return
 	}
 
 	w.WriteHeader(http.StatusOK)
-	w.Write([]byte(fmt.Sprintf("Added %d routes", nRoutesAdded)))
+	w.Write(fmt.Appendf(nil, "Added %d routes", nRoutesAdded))
 }
diff --git a/internal/utils/strutils/filepath.go b/internal/utils/strutils/filepath.go
new file mode 100644
index 0000000..86afe0e
--- /dev/null
+++ b/internal/utils/strutils/filepath.go
@@ -0,0 +1,11 @@
+package strutils
+
+import "strings"
+
+// IsValidFilename checks if a filename is safe and doesn't contain path traversal attempts
+// Returns true if the filename is valid, false otherwise
+func IsValidFilename(filename string) bool {
+	return !strings.Contains(filename, "/") &&
+		!strings.Contains(filename, "\\") &&
+		!strings.Contains(filename, "..")
+}
diff --git a/internal/utils/strutils/url.go b/internal/utils/strutils/url.go
new file mode 100644
index 0000000..4f913a1
--- /dev/null
+++ b/internal/utils/strutils/url.go
@@ -0,0 +1,20 @@
+package strutils
+
+import "path"
+
+// SanitizeURI sanitizes a URI reference to ensure it is safe
+// It disallows URLs beginning with // or /\ as absolute URLs,
+// cleans the URL path to remove any .. or . path elements,
+// and ensures the URL starts with a / if it doesn't already
+func SanitizeURI(uri string) string {
+	if uri == "" {
+		return "/"
+	}
+	if uri[0] != '/' {
+		uri = "/" + uri
+	}
+	if len(uri) > 1 && uri[0] == '/' && uri[1] != '/' && uri[1] != '\\' {
+		return path.Clean(uri)
+	}
+	return "/"
+}
diff --git a/internal/utils/strutils/url_test.go b/internal/utils/strutils/url_test.go
new file mode 100644
index 0000000..8143991
--- /dev/null
+++ b/internal/utils/strutils/url_test.go
@@ -0,0 +1,63 @@
+package strutils
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/require"
+)
+
+func TestSanitizeURI(t *testing.T) {
+	tests := []struct {
+		name     string
+		input    string
+		expected string
+	}{
+		{
+			name:     "empty string",
+			input:    "",
+			expected: "/",
+		},
+		{
+			name:     "single slash",
+			input:    "/",
+			expected: "/",
+		},
+		{
+			name:     "normal path",
+			input:    "/path/to/resource",
+			expected: "/path/to/resource",
+		},
+		{
+			name:     "path without leading slash",
+			input:    "path/to/resource",
+			expected: "/path/to/resource",
+		},
+		{
+			name:     "path with dot segments",
+			input:    "/path/./to/../resource",
+			expected: "/path/resource",
+		},
+		{
+			name:     "double slash prefix",
+			input:    "//path/to/resource",
+			expected: "/",
+		},
+		{
+			name:     "backslash prefix",
+			input:    "/\\path/to/resource",
+			expected: "/",
+		},
+		{
+			name:     "path with multiple slashes",
+			input:    "/path//to///resource",
+			expected: "/path/to/resource",
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			result := SanitizeURI(tt.input)
+			require.Equal(t, tt.expected, result)
+		})
+	}
+}