Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove all HTML tags from comment title and username #1680

Merged
merged 1 commit into from
Oct 10, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 3 additions & 13 deletions backend/app/rest/api/rest_public_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -493,16 +493,10 @@ func TestRest_FindUserComments_CWE_918(t *testing.T) {

arbitraryURLComment := store.Comment{Text: "arbitrary URL request test",
Locator: store.Locator{SiteID: "remark42", URL: arbitraryServer.URL}}
aHrefTitleComment := store.Comment{Text: "a href title test", PostTitle: "<a href=\"https://example.com\">test</a>",
Locator: store.Locator{SiteID: "remark42", URL: "https://radio-t.com/blah1"}}
urlTitleComment := store.Comment{Text: "url title test", PostTitle: "https://j5pxshabxb5037lms6z182pkjbp4d01p.oastify.com",
Locator: store.Locator{SiteID: "remark42", URL: "https://radio-t.com/blah2"}}

assert.False(t, backendRequestedArbitraryServer)
addComment(t, arbitraryURLComment, ts)
assert.True(t, backendRequestedArbitraryServer)
addComment(t, aHrefTitleComment, ts)
addComment(t, urlTitleComment, ts)

res, code := get(t, ts.URL+"/api/v1/comments?site=remark42&user=provider1_dev")
assert.Equal(t, http.StatusOK, code)
Expand All @@ -514,14 +508,10 @@ func TestRest_FindUserComments_CWE_918(t *testing.T) {

err := json.Unmarshal([]byte(res), &resp)
assert.NoError(t, err)
require.Equal(t, 3, len(resp.Comments), "should have 2 comments")
require.Equal(t, 1, len(resp.Comments), "should have 2 comments")

assert.Equal(t, "https://j5pxshabxb5037lms6z182pkjbp4d01p.oastify.com", resp.Comments[0].PostTitle, "unsanitised post title")
assert.Equal(t, "https://radio-t.com/blah2", resp.Comments[0].Locator.URL)
assert.Equal(t, "&lt;a href=\"https://example.com\" rel=\"nofollow\"&gt;test&lt;/a&gt;", resp.Comments[1].PostTitle, "unsanitised post title")
assert.Equal(t, "https://radio-t.com/blah1", resp.Comments[1].Locator.URL)
assert.Equal(t, "", resp.Comments[2].PostTitle, "empty from the first post")
assert.Equal(t, arbitraryServer.URL, resp.Comments[2].Locator.URL, "arbitrary URL provided by the request")
assert.Equal(t, "", resp.Comments[0].PostTitle, "empty from the first post")
assert.Equal(t, arbitraryServer.URL, resp.Comments[0].Locator.URL, "arbitrary URL provided by the request")
}

func TestRest_UserInfo(t *testing.T) {
Expand Down
6 changes: 3 additions & 3 deletions backend/app/store/comment.go
Original file line number Diff line number Diff line change
Expand Up @@ -182,8 +182,8 @@ func (c *Comment) escapeHTMLWithSome(inp string) string {
return res
}

// SanitizeText used to sanitize any input string
// SanitizeText used to sanitize any input string, and removes any HTML tags
func (c *Comment) SanitizeText(inp string) string {
clean := bluemonday.UGCPolicy().Sanitize(inp)
return c.escapeHTMLWithSome(clean)
clean := bluemonday.StrictPolicy().Sanitize(inp)
return strings.TrimSpace(c.escapeHTMLWithSome(clean))
}
12 changes: 10 additions & 2 deletions backend/app/store/comment_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ func TestComment_Sanitize(t *testing.T) {
},
out: Comment{
Text: "blah XSS\n\t",
User: User{ID: `&lt;a href=&#34;http://blah.com&#34;&gt;username&lt;/a&gt;`, Name: "name &lt;b/&gt;"},
User: User{ID: `&lt;a href=&#34;http://blah.com&#34;&gt;username&lt;/a&gt;`, Name: "name"},
},
},
{
Expand Down Expand Up @@ -88,6 +88,14 @@ func TestComment_Sanitize(t *testing.T) {
inp: Comment{Text: "blah blah", PostTitle: "<script>alert()</script>something"},
out: Comment{Text: "blah blah", PostTitle: "something"},
},
{
inp: Comment{Text: "blah blah", PostTitle: "<a href=\"https://example.com\">test</a>"},
out: Comment{Text: "blah blah", PostTitle: "test"},
},
{
inp: Comment{Text: "blah blah", PostTitle: "https://example.com/blah"}, // link is left as-is, but not rendered as <a>
out: Comment{Text: "blah blah", PostTitle: "https://example.com/blah"},
},
{
inp: Comment{Text: `<blockquote class="twitter-tweet"><p lang="es" dir="ltr">Silicon iMac Concept<a href="https://t.co/7ga95QxVXn">https://t.co/7ga95QxVXn</a> by <a href="https://twitter.com/marcsheep?ref_src=twsrc%5Etfw">@marcsheep</a> <a href="https://t.co/ULnVpG8w55">pic.twitter.com/ULnVpG8w55</a></p>&mdash; Andreas Storm (@avstorm) <a href="https://twitter.com/avstorm/status/1325693387798933504?ref_src=twsrc%5Etfw">November 9, 2020</a></blockquote> <script async src="https://platform.twitter.com/widgets.js" charset="utf-8"></script>`, PostTitle: "Twitter quote"},
out: Comment{Text: `<blockquote class="twitter-tweet"><p lang="es" dir="ltr">Silicon iMac Concept<a href="https://t.co/7ga95QxVXn" rel="nofollow">https://t.co/7ga95QxVXn</a> by <a href="https://twitter.com/marcsheep?ref_src=twsrc%5Etfw" rel="nofollow">@marcsheep</a> <a href="https://t.co/ULnVpG8w55" rel="nofollow">pic.twitter.com/ULnVpG8w55</a></p>— Andreas Storm (@avstorm) <a href="https://twitter.com/avstorm/status/1325693387798933504?ref_src=twsrc%5Etfw" rel="nofollow">November 9, 2020</a></blockquote> `, PostTitle: "Twitter quote"},
Expand Down Expand Up @@ -263,7 +271,7 @@ func TestComment_sanitizeText(t *testing.T) {
},
{
"<a href=javascript:alert(document.domain)//>xxx</a>",
"xxx&lt;/a&gt;",
"xxx",
},
}

Expand Down
2 changes: 1 addition & 1 deletion backend/app/store/service/service_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1156,7 +1156,7 @@ func TestService_Find(t *testing.T) {
assert.InDelta(t, 0, res[1].Controversy, 0.01)

// make sure title sanitized
assert.Equal(t, "some title, &lt;a href=\"http://radio-t.com\" rel=\"nofollow\"&gt;link&lt;/a&gt;", res[0].PostTitle)
assert.Equal(t, "some title, link", res[0].PostTitle)
}

func TestService_FindSince(t *testing.T) {
Expand Down