Fix forwarded proto handling for public URL detection

modules/httplib/url.go

@@ -46,14 +46,14 @@ func IsRelativeURL(s string) bool {
 
 func getRequestScheme(req *http.Request) string {
 	// https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/X-Forwarded-Proto
-	if s := req.Header.Get("X-Forwarded-Proto"); s != "" {
-		return s
+	if proto, ok := parseForwardedProtoValue(req.Header.Get("X-Forwarded-Proto")); ok {
+		return proto
 	}
-	if s := req.Header.Get("X-Forwarded-Protocol"); s != "" {
-		return s
+	if proto, ok := parseForwardedProtoValue(req.Header.Get("X-Forwarded-Protocol")); ok {
+		return proto
 	}
-	if s := req.Header.Get("X-Url-Scheme"); s != "" {
-		return s
+	if proto, ok := parseForwardedProtoValue(req.Header.Get("X-Url-Scheme")); ok {
+		return proto
 	}
 	if s := req.Header.Get("Front-End-Https"); s != "" {
 		return util.Iif(s == "on", "https", "http")
@@ -64,6 +64,13 @@ func getRequestScheme(req *http.Request) string {
 	return ""
 }
 
+func parseForwardedProtoValue(val string) (string, bool) {
+	if val == "http" || val == "https" {
+		return val, true
+	}
+	return "", false
+}
+
 // GuessCurrentAppURL tries to guess the current full public URL (with sub-path) by http headers. It always has a '/' suffix, exactly the same as setting.AppURL
 // TODO: should rename it to GuessCurrentPublicURL in the future
 func GuessCurrentAppURL(ctx context.Context) string {

modules/httplib/url_test.go

@@ -47,6 +47,7 @@ func TestGuessCurrentHostURL(t *testing.T) {
 	defer test.MockVariableValue(&setting.AppURL, "http://cfg-host/sub/")()
 	defer test.MockVariableValue(&setting.AppSubURL, "/sub")()
 	headersWithProto := http.Header{"X-Forwarded-Proto": {"https"}}
+	maliciousProtoHeaders := http.Header{"X-Forwarded-Proto": {"http://attacker.host/?trash="}}
 
 	t.Run("Legacy", func(t *testing.T) {
 		defer test.MockVariableValue(&setting.PublicURLDetection, setting.PublicURLLegacy)()
@@ -60,6 +61,9 @@ func TestGuessCurrentHostURL(t *testing.T) {
 		// if "X-Forwarded-Proto" exists, then use it and "Host" header
 		ctx = context.WithValue(t.Context(), RequestContextKey, &http.Request{Host: "req-host:3000", Header: headersWithProto})
 		assert.Equal(t, "https://req-host:3000", GuessCurrentHostURL(ctx))
+
+		ctx = context.WithValue(t.Context(), RequestContextKey, &http.Request{Host: "req-host:3000", Header: maliciousProtoHeaders})
+		assert.Equal(t, "http://cfg-host", GuessCurrentHostURL(ctx))
 	})
 
 	t.Run("Auto", func(t *testing.T) {
@@ -76,6 +80,9 @@ func TestGuessCurrentHostURL(t *testing.T) {
 
 		ctx = context.WithValue(t.Context(), RequestContextKey, &http.Request{Host: "req-host:3000", Header: headersWithProto})
 		assert.Equal(t, "https://req-host:3000", GuessCurrentHostURL(ctx))
+
+		ctx = context.WithValue(t.Context(), RequestContextKey, &http.Request{Host: "req-host:3000", Header: maliciousProtoHeaders})
+		assert.Equal(t, "http://req-host:3000", GuessCurrentHostURL(ctx))
 	})
 
 	t.Run("Never", func(t *testing.T) {