[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.
This commit is contained in:
parent
565e331238
commit
ca798e4cc2
|
@ -619,7 +619,7 @@
|
||||||
{{else}}
|
{{else}}
|
||||||
{{$reviewerName = .Review.OriginalAuthor}}
|
{{$reviewerName = .Review.OriginalAuthor}}
|
||||||
{{end}}
|
{{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>
|
</span>
|
||||||
</div>
|
</div>
|
||||||
{{if .Content}}
|
{{if .Content}}
|
||||||
|
|
|
@ -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
|
|
@ -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
|
|
@ -13,6 +13,7 @@ import (
|
||||||
"testing"
|
"testing"
|
||||||
"time"
|
"time"
|
||||||
|
|
||||||
|
issues_model "code.gitea.io/gitea/models/issues"
|
||||||
"code.gitea.io/gitea/models/unittest"
|
"code.gitea.io/gitea/models/unittest"
|
||||||
user_model "code.gitea.io/gitea/models/user"
|
user_model "code.gitea.io/gitea/models/user"
|
||||||
"code.gitea.io/gitea/modules/git"
|
"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`)
|
||||||
|
}
|
||||||
|
|
Loading…
Reference in a new issue