Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion main.go
Original file line number Diff line number Diff line change
Expand Up @@ -705,7 +705,10 @@ func normalizeDestination(destination string) string {
return "/"
}

unescaped, err := url.QueryUnescape(destination)
// The form parser has already applied application/x-www-form-urlencoded
// decoding. Use path semantics for destinations that are still escaped so
// literal plus signs are not decoded a second time as spaces.
unescaped, err := url.PathUnescape(destination)
if err == nil && unescaped != destination {
if sanitized := sanitizeDestination(unescaped); sanitized != "/" || unescaped == "/" {
return sanitized
Expand Down
52 changes: 37 additions & 15 deletions main_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1211,12 +1211,13 @@ func TestStateBookkeepingErrorBranches(t *testing.T) {

func TestVerifyChallengePage(t *testing.T) {
tests := []struct {
name string
provider string
formValues map[string]string
mockResponse string
expectedStatus int
shouldSetCache bool
name string
provider string
formValues map[string]string
mockResponse string
expectedStatus int
shouldSetCache bool
expectedLocation string
}{
{
name: "Missing captcha response",
Expand All @@ -1232,19 +1233,33 @@ func TestVerifyChallengePage(t *testing.T) {
"cf-turnstile-response": "valid-token",
"destination": "%2Fhome",
},
mockResponse: `{"success":true}`,
expectedStatus: http.StatusFound,
shouldSetCache: true,
mockResponse: `{"success":true}`,
expectedStatus: http.StatusFound,
shouldSetCache: true,
expectedLocation: "/home",
},
{
name: "Successful verification preserves literal plus",
provider: "turnstile",
formValues: map[string]string{
"cf-turnstile-response": "valid-token",
"destination": "/Chrome%20+%20MariaDB%20",
},
mockResponse: `{"success":true}`,
expectedStatus: http.StatusFound,
shouldSetCache: true,
expectedLocation: "/Chrome%20+%20MariaDB%20",
},
{
name: "Successful verification without destination",
provider: "recaptcha",
formValues: map[string]string{
"g-recaptcha-response": "valid-token",
},
mockResponse: `{"success":true}`,
expectedStatus: http.StatusFound,
shouldSetCache: true,
mockResponse: `{"success":true}`,
expectedStatus: http.StatusFound,
shouldSetCache: true,
expectedLocation: "/",
},
{
name: "Failed verification",
Expand All @@ -1263,9 +1278,10 @@ func TestVerifyChallengePage(t *testing.T) {
"cf-turnstile-response": "valid-token",
"destination": "%ZZ",
},
mockResponse: `{"success":true}`,
expectedStatus: http.StatusFound,
shouldSetCache: true,
mockResponse: `{"success":true}`,
expectedStatus: http.StatusFound,
shouldSetCache: true,
expectedLocation: "/",
},
}

Expand Down Expand Up @@ -1310,6 +1326,10 @@ func TestVerifyChallengePage(t *testing.T) {
if found != tt.shouldSetCache {
t.Errorf("Expected cache set=%v, got=%v", tt.shouldSetCache, found)
}

if tt.expectedLocation != "" && rr.Header().Get("Location") != tt.expectedLocation {
t.Errorf("Expected Location %q, got %q", tt.expectedLocation, rr.Header().Get("Location"))
}
})
}
}
Expand Down Expand Up @@ -2106,6 +2126,8 @@ func TestNormalizeDestination(t *testing.T) {
{name: "empty", destination: "", want: "/"},
{name: "decoded local path", destination: "/home?foo=bar", want: "/home?foo=bar"},
{name: "encoded local path", destination: "%2Fhome%3Ffoo%3Dbar", want: "/home?foo=bar"},
{name: "decoded path with literal plus", destination: "/Chrome%20+%20MariaDB%20", want: "/Chrome%20+%20MariaDB%20"},
{name: "encoded path with literal plus", destination: "%2FChrome%2520%2B%2520MariaDB%2520", want: "/Chrome%20+%20MariaDB%20"},
{name: "absolute url", destination: "https://evil.com/phish", want: "/"},
{name: "protocol relative url", destination: "//evil.com/phish", want: "/"},
{name: "relative path", destination: "home", want: "/"},
Expand Down