peezy-tech/jojo
2
0
Fork 0
mirror of https://codeberg.org/forgejo/forgejo.git synced 2026-05-12 22:10:25 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions

fix: consider more risky redirects

Browse source 
After evaluating `..`, it's possible that the resulting path is a risky
redirect, as it might be a "browser" special redirect. Detect this case.
This commit is contained in:
Gusted 2026-02-20 18:10:49 +01:00 • committed by Mathieu Fenniak
parent b55d819a91
commit af0173894a
2 changed files with 21 additions and 5 deletions
Download patch file Download diff file Expand all files Collapse all files

19
modules/httplib/url.go
View file

@ -1,20 +1,26 @@
// Copyright 2023 The Gitea Authors. All rights reserved.
// Copyright 2026 The Forgejo Authors. All rights reserved.
// SPDX-License-Identifier: MIT
package httplib
import (
"net/url"
"path"
"strings"
"forgejo.org/modules/setting"
)
// Unfortunately browsers consider a redirect Location with preceding "//", "\\", "/\" and "\/" as meaning redirect to "http(s)://REST_OF_PATH"
// Therefore we should ignore these redirect locations to prevent open redirects.
func isBrowserRedirect(s string) bool {
return len(s) > 1 && (s[0] == '/' || s[0] == '\\') && (s[1] == '/' || s[1] == '\\')
}
// IsRiskyRedirectURL returns true if the URL is considered risky for redirects
func IsRiskyRedirectURL(s string) bool {
// Unfortunately browsers consider a redirect Location with preceding "//", "\\", "/\" and "\/" as meaning redirect to "http(s)://REST_OF_PATH"
// Therefore we should ignore these redirect locations to prevent open redirects
if len(s) > 1 && (s[0] == '/' || s[0] == '\\') && (s[1] == '/' || s[1] == '\\') {
if isBrowserRedirect(s) {
return true
}
@ -23,5 +29,12 @@ func IsRiskyRedirectURL(s string) bool {
return true
}
// If the path contains `..` then it's still possible this is seen
// as a browser redirect, use `path.Clean` to eliminate each inner `..`
// and then check if that might be a browser redirect.
if strings.Contains(u.Path, "..") {
return isBrowserRedirect(path.Clean(u.Path))
}
return false
}

7
modules/httplib/url_test.go
View file

@ -29,7 +29,6 @@ func TestIsRiskyRedirectURL(t *testing.T) {
{"/sub/foo", false},
{"http://localhost:3000/sub/foo", false},
{"http://localhost:3000/sub/test?param=false", false},
// FIXME: should probably be true (would requires resolving references using setting.appURL.ResolveReference(u))
{"/sub/../", false},
{"http://localhost:3000/sub/../", false},
{"/sUb/", false},
@ -58,10 +57,12 @@ func TestIsRiskyRedirectURL(t *testing.T) {
{"://missing protocol scheme", true},
// FIXME: should probably be false
{"//localhost:3000/sub/test?param=false", true},
{"/a/../\\example.com", true},
{"/a/%2e%2e/\\example.com", true},
}
for _, tt := range tests {
t.Run(tt.input, func(t *testing.T) {
assert.Equal(t, tt.want, IsRiskyRedirectURL(tt.input))
assert.Equal(t, tt.want, IsRiskyRedirectURL(tt.input), tt.input)
})
}
}
@ -114,6 +115,8 @@ func TestIsRiskyRedirectURLWithoutSubURL(t *testing.T) {
{"https://next.forgejo.org", true},
{"//next.forgejo.org/test?param=false", true},
{"//next.forgejo.org/sub/test?param=false", true},
{"/a/../\\example.com", true},
{"/a/%2e%2e/\\example.com", true},
}
for _, tt := range tests {
t.Run(tt.input, func(t *testing.T) {