fix(lib): block XSS attacks via nonstandard URLs (#904)

* fix(lib): block XSS attacks via nonstandard URLs

This could allow an attacker to craft an Anubis pass-challenge URL that
forces a redirect to nonstandard URLs, such as the `javascript:` scheme
which executes arbitrary JavaScript code in a browser context when the
user clicks the "Try again" button.

Release-status: cut
Signed-off-by: Xe Iaso <me@xeiaso.net>

* chore: spelling

Signed-off-by: Xe Iaso <me@xeiaso.net>

---------

Signed-off-by: Xe Iaso <me@xeiaso.net>

.github/actions/spelling/allow.txt

@@ -4,3 +4,4 @@ ssh
 ubuntu
 workarounds
 rjack
+msgbox

.github/actions/spelling/patterns.txt

@@ -132,3 +132,7 @@ go install(?:\s+[a-z]+\.[-@\w/.]+)+
 # hit-count: 1 file-count: 1
 # microsoft
 \b(?:https?://|)(?:(?:(?:blogs|download\.visualstudio|docs|msdn2?|research)\.|)microsoft|blogs\.msdn)\.co(?:m|\.\w\w)/[-_a-zA-Z0-9()=./%]*
+
+# hit-count: 1 file-count: 1
+# data url
+\bdata:[-a-zA-Z=;:/0-9+]*,\S*

docs/docs/CHANGELOG.md

@@ -13,6 +13,14 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 <!-- This changes the project to: -->
 
+### Fixes
+
+#### Fixes a problem with nonstandard URLs and redirects
+
+This could allow an attacker to craft an Anubis pass-challenge URL that forces a redirect to nonstandard URLs, such as the `javascript:` scheme which executes arbitrary JavaScript code in a browser context when the user clicks the "Try again" button.
+
+This has been fixed by disallowing any URLs without the scheme `http` or `https`.
+
 ## v1.21.1: Minfilia Warde - Echo 1
 
 - Expired records are now properly removed from bbolt databases ([#848](https://github.com/TecharoHQ/anubis/pull/848)).

lib/anubis.go

@@ -399,12 +399,20 @@ func (s *Server) PassChallenge(w http.ResponseWriter, r *http.Request) {
 	}
 
 	redir := r.FormValue("redir")
+
 	redirURL, err := url.ParseRequestURI(redir)
 	if err != nil {
 		lg.Error("invalid redirect", "err", err)
 		s.respondWithError(w, r, localizer.T("invalid_redirect"))
 		return
 	}
+
+	if redirURL.Scheme != "" && redirURL.Scheme != "http" && redirURL.Scheme != "https" {
+		lg.Error("XSS attempt blocked, invalid redirect scheme", "scheme", redirURL.Scheme)
+		s.respondWithStatus(w, r, localizer.T("invalid_redirect"), http.StatusBadRequest)
+		return
+	}
+
 	// used by the path checker rule
 	r.URL = redirURL
 

lib/anubis_test.go

@@ -801,3 +801,79 @@ func TestChallengeFor_ErrNotFound(t *testing.T) {
 		}
 	})
 }
+
+func TestPassChallengeXSS(t *testing.T) {
+	pol := loadPolicies(t, "", anubis.DefaultDifficulty)
+
+	srv := spawnAnubis(t, Options{
+		Next:   http.NewServeMux(),
+		Policy: pol,
+	})
+
+	ts := httptest.NewServer(internal.RemoteXRealIP(true, "tcp", srv))
+	defer ts.Close()
+
+	cli := httpClient(t)
+	chall := makeChallenge(t, ts, cli)
+
+	testCases := []struct {
+		name  string
+		redir string
+	}{
+		{
+			name:  "javascript alert",
+			redir: "javascript:alert('xss')",
+		},
+		{
+			name:  "vbscript",
+			redir: "vbscript:msgbox(\"XSS\")",
+		},
+		{
+			name:  "data url",
+			redir: "data:text/html;base64,PHNjcmlwdD5hbGVydCgneHNzJyk8L3NjcmlwdD4=",
+		},
+	}
+
+	for _, tc := range testCases {
+		t.Run(tc.name, func(t *testing.T) {
+			nonce := 0
+			elapsedTime := 420
+			calculated := ""
+			calcString := fmt.Sprintf("%s%d", chall.Challenge, nonce)
+			calculated = internal.SHA256sum(calcString)
+
+			req, err := http.NewRequest(http.MethodGet, ts.URL+"/.within.website/x/cmd/anubis/api/pass-challenge", nil)
+			if err != nil {
+				t.Fatalf("can't make request: %v", err)
+			}
+
+			q := req.URL.Query()
+			q.Set("response", calculated)
+			q.Set("nonce", fmt.Sprint(nonce))
+			q.Set("redir", tc.redir)
+			q.Set("elapsedTime", fmt.Sprint(elapsedTime))
+			req.URL.RawQuery = q.Encode()
+
+			u, err := url.Parse(ts.URL)
+			if err != nil {
+				t.Fatal(err)
+			}
+
+			for _, ckie := range cli.Jar.Cookies(u) {
+				if ckie.Name == anubis.TestCookieName {
+					req.AddCookie(ckie)
+				}
+			}
+
+			resp, err := cli.Do(req)
+			if err != nil {
+				t.Fatalf("can't do request: %v", err)
+			}
+
+			if resp.StatusCode != http.StatusBadRequest {
+				body, _ := io.ReadAll(resp.Body)
+				t.Errorf("wanted status %d, got %d. body: %s", http.StatusBadRequest, resp.StatusCode, body)
+			}
+		})
+	}
+}