From e2517e0fa93177ef6947245eaeab543c95ef18ff Mon Sep 17 00:00:00 2001 From: Giteabot Date: Sat, 7 Mar 2026 03:02:50 +0800 Subject: [PATCH] Fix forwarded proto handling for public URL detection (#36810) (#36836) Backport #36810 by @lunny - normalize `X-Forwarded-Proto`/related headers to accept only `http`/`https` - ignore malformed or injected scheme values to prevent spoofed canonical URLs - add tests covering malicious and multi-valued forwarded proto headers --- Generated by a coding agent with Codex 5.2 Co-authored-by: Lunny Xiao Co-authored-by: wxiaoguang Co-authored-by: silverwind --- modules/httplib/url.go | 19 +++++++++++++------ modules/httplib/url_test.go | 7 +++++++ 2 files changed, 20 insertions(+), 6 deletions(-) diff --git a/modules/httplib/url.go b/modules/httplib/url.go index 5f35e66964..f1fc1d622f 100644 --- a/modules/httplib/url.go +++ b/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 { diff --git a/modules/httplib/url_test.go b/modules/httplib/url_test.go index 4270d9fa89..cfc2659786 100644 --- a/modules/httplib/url_test.go +++ b/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)) }) }