From 1075ff74b5050f671c5f9824ae39390230b3c85d Mon Sep 17 00:00:00 2001
From: Earl Warren <109468362+earl-warren@users.noreply.github.com>
Date: Thu, 23 Nov 2023 17:34:25 +0100
Subject: [PATCH] Use restricted sanitizer for repository description (#28141)

- Currently the repository description uses the same sanitizer as a
normal markdown document. This means that element such as heading and
images are allowed and can be abused.
- Create a minimal restricted sanitizer for the repository description,
which only allows what the postprocessor currently allows, which are
links and emojis.
- Added unit testing.
- Resolves https://codeberg.org/forgejo/forgejo/issues/1202
- Resolves https://codeberg.org/Codeberg/Community/issues/1122

(cherry picked from commit 631c87cc2347f0036a75dcd21e24429bbca28207)

Co-authored-by: Gusted <postmaster@gusted.xyz>
---
 models/repo/repo.go              |  4 ++--
 modules/markup/sanitizer.go      | 35 +++++++++++++++++++++++++++++---
 modules/markup/sanitizer_test.go | 22 ++++++++++++++++++++
 3 files changed, 56 insertions(+), 5 deletions(-)

diff --git a/models/repo/repo.go b/models/repo/repo.go
index c4b215e07..db3709f1e 100644
--- a/models/repo/repo.go
+++ b/models/repo/repo.go
@@ -584,9 +584,9 @@ func (repo *Repository) DescriptionHTML(ctx context.Context) template.HTML {
 	}, repo.Description)
 	if err != nil {
 		log.Error("Failed to render description for %s (ID: %d): %v", repo.Name, repo.ID, err)
-		return template.HTML(markup.Sanitize(repo.Description))
+		return template.HTML(markup.SanitizeDescription(repo.Description))
 	}
-	return template.HTML(markup.Sanitize(desc))
+	return template.HTML(markup.SanitizeDescription(desc))
 }
 
 // CloneLink represents different types of clone URLs of repository.
diff --git a/modules/markup/sanitizer.go b/modules/markup/sanitizer.go
index 48c08831f..992e85b98 100644
--- a/modules/markup/sanitizer.go
+++ b/modules/markup/sanitizer.go
@@ -18,9 +18,10 @@ import (
 // Sanitizer is a protection wrapper of *bluemonday.Policy which does not allow
 // any modification to the underlying policies once it's been created.
 type Sanitizer struct {
-	defaultPolicy    *bluemonday.Policy
-	rendererPolicies map[string]*bluemonday.Policy
-	init             sync.Once
+	defaultPolicy     *bluemonday.Policy
+	descriptionPolicy *bluemonday.Policy
+	rendererPolicies  map[string]*bluemonday.Policy
+	init              sync.Once
 }
 
 var (
@@ -41,6 +42,7 @@ func NewSanitizer() {
 func InitializeSanitizer() {
 	sanitizer.rendererPolicies = map[string]*bluemonday.Policy{}
 	sanitizer.defaultPolicy = createDefaultPolicy()
+	sanitizer.descriptionPolicy = createRepoDescriptionPolicy()
 
 	for name, renderer := range renderers {
 		sanitizerRules := renderer.SanitizerRules()
@@ -161,6 +163,27 @@ func createDefaultPolicy() *bluemonday.Policy {
 	return policy
 }
 
+// createRepoDescriptionPolicy returns a minimal more strict policy that is used for
+// repository descriptions.
+func createRepoDescriptionPolicy() *bluemonday.Policy {
+	policy := bluemonday.NewPolicy()
+
+	// Allow italics and bold.
+	policy.AllowElements("i", "b", "em", "strong")
+
+	// Allow code.
+	policy.AllowElements("code")
+
+	// Allow links
+	policy.AllowAttrs("href", "target", "rel").OnElements("a")
+
+	// Allow classes for emojis
+	policy.AllowAttrs("class").Matching(regexp.MustCompile(`^emoji$`)).OnElements("img", "span")
+	policy.AllowAttrs("aria-label").OnElements("span")
+
+	return policy
+}
+
 func addSanitizerRules(policy *bluemonday.Policy, rules []setting.MarkupSanitizerRule) {
 	for _, rule := range rules {
 		if rule.AllowDataURIImages {
@@ -176,6 +199,12 @@ func addSanitizerRules(policy *bluemonday.Policy, rules []setting.MarkupSanitize
 	}
 }
 
+// SanitizeDescription sanitizes the HTML generated for a repository description.
+func SanitizeDescription(s string) string {
+	NewSanitizer()
+	return sanitizer.descriptionPolicy.Sanitize(s)
+}
+
 // Sanitize takes a string that contains a HTML fragment or document and applies policy whitelist.
 func Sanitize(s string) string {
 	NewSanitizer()
diff --git a/modules/markup/sanitizer_test.go b/modules/markup/sanitizer_test.go
index 0bc63ff0a..b7b8792bd 100644
--- a/modules/markup/sanitizer_test.go
+++ b/modules/markup/sanitizer_test.go
@@ -73,6 +73,28 @@ func Test_Sanitizer(t *testing.T) {
 	}
 }
 
+func TestDescriptionSanitizer(t *testing.T) {
+	NewSanitizer()
+
+	testCases := []string{
+		`<h1>Title</h1>`, `Title`,
+		`<img src='img.png' alt='image'>`, ``,
+		`<span class="emoji" aria-label="thumbs up">THUMBS UP</span>`, `<span class="emoji" aria-label="thumbs up">THUMBS UP</span>`,
+		`<span style="color: red">Hello World</span>`, `<span>Hello World</span>`,
+		`<br>`, ``,
+		`<a href="https://example.com" target="_blank" rel="noopener noreferrer">https://example.com</a>`, `<a href="https://example.com" target="_blank" rel="noopener noreferrer">https://example.com</a>`,
+		`<mark>Important!</mark>`, `Important!`,
+		`<details>Click me! <summary>Nothing to see here.</summary></details>`, `Click me! Nothing to see here.`,
+		`<input type="hidden">`, ``,
+		`<b>I</b> have a <i>strong</i> <strong>opinion</strong> about <em>this</em>.`, `<b>I</b> have a <i>strong</i> <strong>opinion</strong> about <em>this</em>.`,
+		`Provides alternative <code>wg(8)</code> tool`, `Provides alternative <code>wg(8)</code> tool`,
+	}
+
+	for i := 0; i < len(testCases); i += 2 {
+		assert.Equal(t, testCases[i+1], SanitizeDescription(testCases[i]))
+	}
+}
+
 func TestSanitizeNonEscape(t *testing.T) {
 	descStr := "<scrİpt>&lt;script&gt;alert(document.domain)&lt;/script&gt;</scrİpt>"