Skip to content

Commit

Permalink
ADD: Comment HTML sanitization and tests for it (publiclab#7282)
Browse files Browse the repository at this point in the history
Unit tests:
 - "sanitizing comment body for XSS"
 - "should close incomplete tags"

Resolves publiclab#3630 publiclab#3553
  • Loading branch information
VladimirMikulic authored and Vinit Shahdeo committed Feb 1, 2020
1 parent 678b4ab commit 2fc1586
Show file tree
Hide file tree
Showing 3 changed files with 49 additions and 5 deletions.
9 changes: 8 additions & 1 deletion app/models/comment.rb
Original file line number Diff line number Diff line change
Expand Up @@ -473,7 +473,14 @@ def render_body
if !trimmed_content? && parsed.present?
body = parsed[:body] + COMMENT_FILTER + parsed[:boundary] + parsed[:quote]
end
body

allowed_tags = %w(a acronym b strong i em li ul ol h1 h2 h3 h4 h5 h6 blockquote br cite sub sup ins p iframe del hr img input code table thead tbody tr th td span dl dt dd div)

# Sanitize the HTML (remove malicious attributes, unallowed tags...)
sanitized_body = ActionController::Base.helpers.sanitize(body, tags: allowed_tags)

# Properly parse HTML (close incomplete tags...)
Nokogiri::HTML::DocumentFragment.parse(sanitized_body).to_html
end

def self.find_by_tag_and_author(tagname, userid)
Expand Down
6 changes: 3 additions & 3 deletions test/functional/notes_controller_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -677,7 +677,7 @@ def teardown
assert (notes & expected).present?
assert !(notes & questions).present?
end

test 'first note in /liked endpoint should be highest liked' do
get :liked
notes = assigns(:notes)
Expand All @@ -696,15 +696,15 @@ def teardown
actual = notes.first
assert expected == actual.created
end

test 'first three posts in /liked should be sorted by likes' do
get :liked
# gets first notes
notes = assigns(:notes)[0...3]
# sort_by is from lowest to highest so it needs to be reversed
assert notes.sort_by { |note| note.cached_likes }.reverse == notes
end

test 'should choose I18n for notes controller' do
available_testing_locales.each do |lang|
old_controller = @controller
Expand Down
39 changes: 38 additions & 1 deletion test/unit/comment_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -260,12 +260,15 @@ def setup
comment = Comment.new({
comment: "Thank you! On Tuesday, 3 July 2018, 11:20:57 PM IST, RP <[email protected]> wrote: Here you go."
})

parsed = comment.parse_quoted_text
output = comment.render_body

assert_equal "Thank you! ", parsed[:body]
assert_equal "On Tuesday, 3 July 2018, 11:20:57 PM IST, RP <[email protected]> wrote:", parsed[:boundary]
assert_equal " Here you go.", parsed[:quote]
assert_equal "Thank you! ", comment.scrub_quoted_text
assert_equal "Thank you! <!-- @@$$%% Trimmed Content @@$$%% -->On Tuesday, 3 July 2018, 11:20:57 PM IST, RP <[email protected]> wrote: Here you go.", comment.render_body
assert_equal output, "Thank you! On Tuesday, 3 July 2018, 11:20:57 PM IST, RP wrote: Here you go."
end

test 'should give the domain of gmail correctly' do
Expand Down Expand Up @@ -313,4 +316,38 @@ def setup
test 'find comments using tagname and user id' do
assert_equal('Admin comment', Comment.find_by_tag_and_author("awesome", 5).first.comment)
end

test 'sanitizing comment body for XSS' do
comment = Comment.new
comment.comment = "<img src=x onerror=prompt(133)>" # inserting executable javascript into a comment
assert comment.save

output = comment.render_body

# Ensure that malicious attributes have been removed
assert_equal [], output.scan('src=x')
assert_equal [], output.scan('onerror=prompt')

# Ensure all the OK attributes are preserved, for a wide range of comment types:
comment.comment = "<iframe src='/hello' width='100' height='100' border='0'></iframe><p style='color:red;' class='nice' id='cool' title='sweet'></p>"
assert comment.save
output = comment.render_body

assert_equal [], output.scan("src='/hello' width='100' height='100' border='0'")
assert_equal [], output.scan("class='nice' id='cool' title='sweet'")
end

test 'should close incomplete tags' do
comment = Comment.new

# <iframe> is not closed (</iframe>)
comment.comment = "Let’s see how this works with images or an embedded video.\n\n&nbsp;\n\n![](cid:[email protected])\n\n&nbsp;\n\n\\<iframe width=\"560\" height=\"315\" src=\"https://www.youtube.com/embed/Kt\\_MSMpxy7Y\" frameborder=\"0\" allow=\"autoplay; encrypted-media\" allowfullscreen\\>\\\n\n&nbsp;\n\nIs \\ ***markdown** \\* parsed?\n\n&nbsp;\n\n1. 1. Number 1\n\n2. 4. Number 4\n\n3. 3. Number 3\n\n&nbsp;"

assert comment.save
output = comment.render_body

# Make sure that <iframe> is closed (</iframe>)
assert_not_equal [], output.scan('<iframe width="560" height="315" src="https://www.youtube.com/embed/Kt%5C_MSMpxy7Y">\</iframe>')
end

end

0 comments on commit 2fc1586

Please sign in to comment.