From ca798e4cc2a8c6e3d1f2cfed01f47d8b3da9361f Mon Sep 17 00:00:00 2001
From: Gusted <postmaster@gusted.xyz>
Date: Thu, 18 Jan 2024 00:18:39 +0100
Subject: [PATCH] [SECURITY] Test XSS in dismissed review

It's possible for reviews to not be assiocated with users, when they
were migrated from another forge instance. In the migration code,
there's no sanitization check for author names, so they could contain
HTML tags and thus needs to be properely escaped.
---
 templates/repo/issue/view_content/comments.tmpl   |  2 +-
 .../fixtures/TestXSSReviewDismissed/comment.yml   |  9 +++++++++
 .../fixtures/TestXSSReviewDismissed/review.yml    |  8 ++++++++
 tests/integration/xss_test.go                     | 15 +++++++++++++++
 4 files changed, 33 insertions(+), 1 deletion(-)
 create mode 100644 tests/integration/fixtures/TestXSSReviewDismissed/comment.yml
 create mode 100644 tests/integration/fixtures/TestXSSReviewDismissed/review.yml

diff --git a/templates/repo/issue/view_content/comments.tmpl b/templates/repo/issue/view_content/comments.tmpl
index 9e50ee4d94..a4fd97297f 100644
--- a/templates/repo/issue/view_content/comments.tmpl
+++ b/templates/repo/issue/view_content/comments.tmpl
@@ -619,7 +619,7 @@
 						{{else}}
 							{{$reviewerName = .Review.OriginalAuthor}}
 						{{end}}
-						{{ctx.Locale.Tr "repo.issues.review.dismissed" $reviewerName $createdStr | Safe}}
+						<span class="dismissed-message">{{ctx.Locale.Tr "repo.issues.review.dismissed" $reviewerName $createdStr | Safe}}</span>
 					</span>
 				</div>
 				{{if .Content}}
diff --git a/tests/integration/fixtures/TestXSSReviewDismissed/comment.yml b/tests/integration/fixtures/TestXSSReviewDismissed/comment.yml
new file mode 100644
index 0000000000..50162a4e7e
--- /dev/null
+++ b/tests/integration/fixtures/TestXSSReviewDismissed/comment.yml
@@ -0,0 +1,9 @@
+-
+  id: 1000
+  type: 32 # dismiss review
+  poster_id: 2
+  issue_id: 2 # in repo_id 1
+  content: "XSS time!"
+  review_id: 1000
+  created_unix: 1700000000
+  updated_unix: 1700000000
diff --git a/tests/integration/fixtures/TestXSSReviewDismissed/review.yml b/tests/integration/fixtures/TestXSSReviewDismissed/review.yml
new file mode 100644
index 0000000000..56bc08d35f
--- /dev/null
+++ b/tests/integration/fixtures/TestXSSReviewDismissed/review.yml
@@ -0,0 +1,8 @@
+-
+  id: 1000
+  type: 1
+  issue_id: 2
+  original_author: "Otto <script class='evil'>alert('Oh no!')</script>"
+  content: "XSS time!"
+  updated_unix: 1700000000
+  created_unix: 1700000000
diff --git a/tests/integration/xss_test.go b/tests/integration/xss_test.go
index 42ce35150c..acd716c7c7 100644
--- a/tests/integration/xss_test.go
+++ b/tests/integration/xss_test.go
@@ -13,6 +13,7 @@ import (
 	"testing"
 	"time"
 
+	issues_model "code.gitea.io/gitea/models/issues"
 	"code.gitea.io/gitea/models/unittest"
 	user_model "code.gitea.io/gitea/models/user"
 	"code.gitea.io/gitea/modules/git"
@@ -112,3 +113,17 @@ func TestXSSWikiLastCommitInfo(t *testing.T) {
 		})
 	})
 }
+
+func TestXSSReviewDismissed(t *testing.T) {
+	defer tests.AddFixtures("tests/integration/fixtures/TestXSSReviewDismissed/")()
+	defer tests.PrepareTestEnv(t)()
+
+	review := unittest.AssertExistsAndLoadBean(t, &issues_model.Review{ID: 1000})
+
+	req := NewRequest(t, http.MethodGet, fmt.Sprintf("/user2/repo1/pulls/%d", +review.IssueID))
+	resp := MakeRequest(t, req, http.StatusOK)
+	htmlDoc := NewHTMLParser(t, resp.Body)
+
+	htmlDoc.AssertElement(t, "script.evil", false)
+	assert.Contains(t, htmlDoc.Find("#issuecomment-1000 .dismissed-message").Text(), `dismissed Otto <script class='evil'>alert('Oh no!')</script>’s review`)
+}