Disallow dangerous url schemes (#25960)

Regression: #24805
Closes: #25945

- Disallow `javascript`, `vbscript` and `data` (data uri images still
work) url schemes even if all other schemes are allowed
- Fixed older `cbthunderlink` tests

---------

Co-authored-by: delvh <[email protected]>

Author: KN4CK3R

go.mod

@@ -76,7 +76,7 @@ require (
 	github.com/mattn/go-sqlite3 v1.14.17
 	github.com/meilisearch/meilisearch-go v0.25.0
 	github.com/mholt/archiver/v3 v3.5.1
-	github.com/microcosm-cc/bluemonday v1.0.24
+	github.com/microcosm-cc/bluemonday v1.0.25
 	github.com/minio/minio-go/v7 v7.0.60
 	github.com/minio/sha256-simd v1.0.1
 	github.com/msteinert/pam v1.1.0

go.sum

@@ -865,8 +865,8 @@ github.com/mholt/acmez v1.2.0 h1:1hhLxSgY5FvH5HCnGUuwbKY2VQVo8IU7rxXKSnZ7F30=
 github.com/mholt/acmez v1.2.0/go.mod h1:VT9YwH1xgNX1kmYY89gY8xPJC84BFAisjo8Egigt4kE=
 github.com/mholt/archiver/v3 v3.5.1 h1:rDjOBX9JSF5BvoJGvjqK479aL70qh9DIpZCl+k7Clwo=
 github.com/mholt/archiver/v3 v3.5.1/go.mod h1:e3dqJ7H78uzsRSEACH1joayhuSyhnonssnDhppzS1L4=
-github.com/microcosm-cc/bluemonday v1.0.24 h1:NGQoPtwGVcbGkKfvyYk1yRqknzBuoMiUrO6R7uFTPlw=
-github.com/microcosm-cc/bluemonday v1.0.24/go.mod h1:ArQySAMps0790cHSkdPEJ7bGkF2VePWH773hsJNSHf8=
+github.com/microcosm-cc/bluemonday v1.0.25 h1:4NEwSfiJ+Wva0VxN5B8OwMicaJvD8r9tlJWm9rtloEg=
+github.com/microcosm-cc/bluemonday v1.0.25/go.mod h1:ZIOjCQp1OrzBBPIJmfX4qDYFuhU02nx4bn030ixfHLE=
 github.com/miekg/dns v1.0.14/go.mod h1:W1PPwlIAgtquWBMBEV9nkV9Cazfe8ScdGz/Lj7v3Nrg=
 github.com/miekg/dns v1.1.55 h1:GoQ4hpsj0nFLYe+bWiCToyrBEJXkQfOOIvFGFy0lEgo=
 github.com/miekg/dns v1.1.55/go.mod h1:uInx36IzPl7FYnDcMeVWxj9byh7DutNykX4G9Sj60FY=

modules/markup/sanitizer.go

@@ -6,6 +6,7 @@ package markup
 
 import (
 	"io"
+	"net/url"
 	"regexp"
 	"sync"
 
@@ -79,6 +80,14 @@ func createDefaultPolicy() *bluemonday.Policy {
 		policy.AllowURLSchemes(setting.Markdown.CustomURLSchemes...)
 	} else {
 		policy.AllowURLSchemesMatching(allowAllRegex)
+
+		// Even if every scheme is allowed, these three are blocked for security reasons
+		disallowScheme := func(*url.URL) bool {
+			return false
+		}
+		policy.AllowURLSchemeWithCustomPolicy("javascript", disallowScheme)
+		policy.AllowURLSchemeWithCustomPolicy("vbscript", disallowScheme)
+		policy.AllowURLSchemeWithCustomPolicy("data", disallowScheme)
 	}
 
 	// Allow classes for anchors

modules/markup/sanitizer_test.go

@@ -54,8 +54,13 @@ func Test_Sanitizer(t *testing.T) {
 		`<code style="bad-color: red">Hello World</code>`, `<code>Hello World</code>`,
 
 		// URLs
-		`[my custom URL scheme](cbthunderlink://somebase64string)`, `[my custom URL scheme](cbthunderlink://somebase64string)`,
-		`[my custom URL scheme](matrix:roomid/psumPMeAfzgAeQpXMG:feneas.org?action=join)`, `[my custom URL scheme](matrix:roomid/psumPMeAfzgAeQpXMG:feneas.org?action=join)`,
+		`<a href="cbthunderlink://somebase64string)">my custom URL scheme</a>`, `<a href="cbthunderlink://somebase64string)" rel="nofollow">my custom URL scheme</a>`,
+		`<a href="matrix:roomid/psumPMeAfzgAeQpXMG:feneas.org?action=join">my custom URL scheme</a>`, `<a href="matrix:roomid/psumPMeAfzgAeQpXMG:feneas.org?action=join" rel="nofollow">my custom URL scheme</a>`,
+
+		// Disallow dangerous url schemes
+		`<a href="javascript:alert('xss')">bad</a>`, `bad`,
+		`<a href="vbscript:no">bad</a>`, `bad`,
+		`<a href="data:1234">bad</a>`, `bad`,
 	}
 
 	for i := 0; i < len(testCases); i += 2 {