fix(lib): add comprehensive XSS protection logic

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

.github/actions/spelling/allow.txt

@@ -3,4 +3,5 @@ https
 ssh
 ubuntu
 workarounds
-rjack
+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*

VERSION

@@ -1 +1 @@
-1.21.1
+1.21.2

docs/docs/CHANGELOG.md

@@ -13,6 +13,16 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 <!-- This changes the project to: -->
 
+## v1.21.2: Minfilia Warde - Echo 2
+
+### 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

@@ -264,7 +264,7 @@ func (s *Server) checkRules(w http.ResponseWriter, r *http.Request, cr policy.Ch
 		hash := rule.Hash()
 
 		lg.Debug("rule hash", "hash", hash)
-		s.respondWithStatus(w, r, fmt.Sprintf("%s %s", localizer.T("access_denied"), hash), s.policy.StatusCodes.Deny)
+		s.respondWithStatus(w, r, fmt.Sprintf("%s %s", localizer.T("access_denied"), hash), "/", s.policy.StatusCodes.Deny)
 		return true
 	case config.RuleChallenge:
 		lg.Debug("challenge requested")
@@ -302,7 +302,7 @@ func (s *Server) handleDNSBL(w http.ResponseWriter, r *http.Request, ip string,
 				localizer.T("dronebl_entry"),
 				resp.String(),
 				localizer.T("see_dronebl_lookup"),
-				ip), s.policy.StatusCodes.Deny)
+				ip), "/", s.policy.StatusCodes.Deny)
 			return true
 		}
 	}
@@ -384,6 +384,23 @@ func (s *Server) PassChallenge(w http.ResponseWriter, r *http.Request) {
 	lg := internal.GetRequestLogger(r)
 	localizer := localization.GetLocalizer(r)
 
+	redir := r.FormValue("redir")
+	redirURL, err := url.ParseRequestURI(redir)
+	if err != nil {
+		lg.Error("invalid redirect", "err", err)
+		s.respondWithStatus(w, r, localizer.T("invalid_redirect"), "/", http.StatusBadRequest)
+		return
+	}
+
+	switch redirURL.Scheme {
+	case "", "http", "https":
+		// allowed
+	default:
+		lg.Error("XSS attempt blocked, invalid redirect scheme", "scheme", redirURL.Scheme)
+		s.respondWithStatus(w, r, localizer.T("invalid_redirect"), "/", http.StatusBadRequest)
+		return
+	}
+
 	// Adjust cookie path if base prefix is not empty
 	cookiePath := "/"
 	if anubis.BasePrefix != "" {
@@ -398,13 +415,6 @@ func (s *Server) PassChallenge(w http.ResponseWriter, r *http.Request) {
 		return
 	}
 
-	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
-	}
 	// used by the path checker rule
 	r.URL = redirURL
 
@@ -456,7 +466,7 @@ func (s *Server) PassChallenge(w http.ResponseWriter, r *http.Request) {
 		case errors.As(err, &cerr):
 			switch {
 			case errors.Is(err, challenge.ErrFailed):
-				s.respondWithStatus(w, r, cerr.PublicReason, cerr.StatusCode)
+				s.respondWithStatus(w, r, cerr.PublicReason, "/", cerr.StatusCode)
 			case errors.Is(err, challenge.ErrInvalidFormat), errors.Is(err, challenge.ErrMissingField):
 				s.respondWithError(w, r, cerr.PublicReason)
 			}

lib/anubis_test.go

@@ -1,6 +1,7 @@
 package lib
 
 import (
+	"bytes"
 	"encoding/json"
 	"fmt"
 	"io"
@@ -801,3 +802,127 @@ 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=",
+		},
+	}
+
+	t.Run("with test cookie", func(t *testing.T) {
+		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)
+				}
+
+				body, _ := io.ReadAll(resp.Body)
+
+				if bytes.Contains(body, []byte(tc.redir)) {
+					t.Log(string(body))
+					t.Error("found XSS in HTML body")
+				}
+
+				if resp.StatusCode != http.StatusBadRequest {
+					t.Errorf("wanted status %d, got %d. body: %s", http.StatusBadRequest, resp.StatusCode, body)
+				}
+			})
+		}
+	})
+
+	t.Run("no test cookie", func(t *testing.T) {
+		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()
+
+				resp, err := cli.Do(req)
+				if err != nil {
+					t.Fatalf("can't do request: %v", err)
+				}
+
+				body, _ := io.ReadAll(resp.Body)
+
+				if bytes.Contains(body, []byte(tc.redir)) {
+					t.Log(string(body))
+					t.Error("found XSS in HTML body")
+				}
+
+				if resp.StatusCode != http.StatusBadRequest {
+					t.Errorf("wanted status %d, got %d. body: %s", http.StatusBadRequest, resp.StatusCode, body)
+				}
+			})
+		}
+	})
+}

lib/http.go

@@ -192,13 +192,13 @@ func (s *Server) RenderBench(w http.ResponseWriter, r *http.Request) {
 }
 
 func (s *Server) respondWithError(w http.ResponseWriter, r *http.Request, message string) {
-	s.respondWithStatus(w, r, message, http.StatusInternalServerError)
+	s.respondWithStatus(w, r, message, "/", http.StatusInternalServerError)
 }
 
-func (s *Server) respondWithStatus(w http.ResponseWriter, r *http.Request, msg string, status int) {
+func (s *Server) respondWithStatus(w http.ResponseWriter, r *http.Request, msg, redirect string, status int) {
 	localizer := localization.GetLocalizer(r)
 
-	templ.Handler(web.Base(localizer.T("oh_noes"), web.ErrorPage(msg, s.opts.WebmasterEmail, r.FormValue("redir"), localizer), s.policy.Impressum, localizer), templ.WithStatus(status)).ServeHTTP(w, r)
+	templ.Handler(web.Base(localizer.T("oh_noes"), web.ErrorPage(msg, s.opts.WebmasterEmail, redirect, localizer), s.policy.Impressum, localizer), templ.WithStatus(status)).ServeHTTP(w, r)
 }
 
 func (s *Server) ServeHTTP(w http.ResponseWriter, r *http.Request) {
@@ -238,12 +238,12 @@ func (s *Server) ServeHTTPNext(w http.ResponseWriter, r *http.Request) {
 		redir := r.FormValue("redir")
 		urlParsed, err := r.URL.Parse(redir)
 		if err != nil {
-			s.respondWithStatus(w, r, localizer.T("redirect_not_parseable"), http.StatusBadRequest)
+			s.respondWithStatus(w, r, localizer.T("redirect_not_parseable"), "/", http.StatusBadRequest)
 			return
 		}
 
 		if (len(urlParsed.Host) > 0 && len(s.opts.RedirectDomains) != 0 && !slices.Contains(s.opts.RedirectDomains, urlParsed.Host)) || urlParsed.Host != r.URL.Host {
-			s.respondWithStatus(w, r, localizer.T("redirect_domain_not_allowed"), http.StatusBadRequest)
+			s.respondWithStatus(w, r, localizer.T("redirect_domain_not_allowed"), "/", http.StatusBadRequest)
 			return
 		}
 

package-lock.json

@@ -1,12 +1,12 @@
 {
   "name": "@techaro/anubis",
-  "version": "1.21.1",
+  "version": "1.21.2",
   "lockfileVersion": 3,
   "requires": true,
   "packages": {
     "": {
       "name": "@techaro/anubis",
-      "version": "1.21.1",
+      "version": "1.21.2",
       "license": "ISC",
       "devDependencies": {
         "cssnano": "^7.1.0",

package.json

@@ -1,6 +1,6 @@
 {
   "name": "@techaro/anubis",
-  "version": "1.21.1",
+  "version": "1.21.2",
   "description": "",
   "main": "index.js",
   "scripts": {