Monorepo for Tangled
tangled.org
appview/state: fix open redirect via return_url after OAuth login
Validate return_url before storing it in the session: only relative
paths starting with "/" (and not "//") are accepted. Anything else —
absolute URLs and protocol-relative URLs — is replaced with "/".
Add tests covering the accepted and rejected cases.
Signed-off-by: Matías Insaurralde <matias@insaurral.de>
authored by
Matías Insaurralde
and committed by
tangled.org
1b29c32f
df56d3db
+39
-1
+11
-1
appview/state/login.go
+11
-1
appview/state/login.go
···
106
106
}
107
107
}
108
108
109
-
if err := s.oauth.SetAuthReturn(w, r, returnURL, addAccount); err != nil {
109
+
if err := s.oauth.SetAuthReturn(w, r, sanitizeReturnURL(returnURL), addAccount); err != nil {
110
110
l.Error("failed to set auth return", "err", err)
111
111
}
112
112
···
123
123
124
124
s.pages.HxRedirect(w, redirectURL)
125
125
}
126
+
}
127
+
128
+
// sanitizeReturnURL ensures the return URL is a relative path on the same
129
+
// origin. Anything else — absolute URLs, protocol-relative URLs — is replaced
130
+
// with "/" to prevent open redirect after OAuth login.
131
+
func sanitizeReturnURL(s string) string {
132
+
if strings.HasPrefix(s, "/") && !strings.HasPrefix(s, "//") {
133
+
return s
134
+
}
135
+
return "/"
126
136
}
127
137
128
138
func (s *State) Logout(w http.ResponseWriter, r *http.Request) {
+28
appview/state/login_test.go
+28
appview/state/login_test.go
···
1
+
package state
2
+
3
+
import "testing"
4
+
5
+
func TestSanitizeReturnURL(t *testing.T) {
6
+
cases := []struct {
7
+
input string
8
+
want string
9
+
}{
10
+
{"/", "/"},
11
+
{"/some/path", "/some/path"},
12
+
{"/valid?query=1", "/valid?query=1"},
13
+
{"/valid#anchor", "/valid#anchor"},
14
+
// External URLs must be rejected.
15
+
{"https://evil.com", "/"},
16
+
{"http://evil.com", "/"},
17
+
// Protocol-relative URLs are treated as external by browsers.
18
+
{"//evil.com", "/"},
19
+
{"//evil.com/phishing", "/"},
20
+
// Empty string.
21
+
{"", "/"},
22
+
}
23
+
for _, tc := range cases {
24
+
if got := sanitizeReturnURL(tc.input); got != tc.want {
25
+
t.Errorf("sanitizeReturnURL(%q) = %q, want %q", tc.input, got, tc.want)
26
+
}
27
+
}
28
+
}