From 672caa68138acf85e681e54242a8c95160869737 Mon Sep 17 00:00:00 2001 From: Gusted 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. (cherry picked from commit ca798e4cc2a8c6e3d1f2cfed01f47d8b3da9361f) (cherry picked from commit d3de80b9cc684d88ffa90f22af9f2dc912af6979) --- .../fixtures/TestXSSReviewDismissed/comment.yml | 9 +++++++++ .../fixtures/TestXSSReviewDismissed/review.yml | 8 ++++++++ tests/integration/xss_test.go | 15 +++++++++++++++ 3 files changed, 32 insertions(+) create mode 100644 tests/integration/fixtures/TestXSSReviewDismissed/comment.yml create mode 100644 tests/integration/fixtures/TestXSSReviewDismissed/review.yml 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 " + 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 ’s review`) +}