From 17ef5cb9a529c1fe2c5fbcea1cecbbb614dd2648 Mon Sep 17 00:00:00 2001
From: yusing <yusing@6uo.me>
Date: Sat, 22 Mar 2025 23:58:37 +0800
Subject: [PATCH] security: sanitize uri

---
 internal/api/v1/favicon/favicon.go  | 13 ++----
 internal/utils/strutils/url.go      | 20 +++++++++
 internal/utils/strutils/url_test.go | 63 +++++++++++++++++++++++++++++
 3 files changed, 87 insertions(+), 9 deletions(-)
 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 53766445..00525dd7 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/http"
 	"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/utils/strutils/url.go b/internal/utils/strutils/url.go
new file mode 100644
index 00000000..4f913a1a
--- /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 00000000..8143991a
--- /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)
+		})
+	}
+}