Skip to content
Permalink
Browse files Browse the repository at this point in the history
Tags with invalid names should also be stripped in order to prevent
XSS attacks.  Thanks Sascha Depold for the report.
  • Loading branch information
tenderlove committed Aug 16, 2011
1 parent 8a39f41 commit 586a944
Show file tree
Hide file tree
Showing 2 changed files with 8 additions and 1 deletion.
Expand Up @@ -156,7 +156,7 @@ def parse(parent, line, pos, content, strict=true)
end

closing = ( scanner.scan(/\//) ? :close : nil )
return Text.new(parent, line, pos, content) unless name = scanner.scan(/[\w:-]+/)
return Text.new(parent, line, pos, content) unless name = scanner.scan(/[^\s!>\/]+/)

This comment has been minimized.

Copy link
@lardawge

lardawge Aug 29, 2011

Contributor

@tenderlove

This breaks passing xml: true to HTML::Document.new

xml = <<- XML
<?xml version="1.0" standalone="yes"?> 
<TRIVIA> 

  <MATH>
    <QUESTION>What is the square root of 25</QUESTION>
    <ANSWER>5</ANSWER>
  </MATH>

  <GENERAL>
    <QUESTION>What is the season after Summer </QUESTION>
    <ANSWER>Fall</ANSWER>
    <ANSWER>Autumn </ANSWER>
  </GENERAL>

</TRIVIA>
XML

f = HTML::Document.new xml, true, true

Resulting in:
RuntimeError: expected > (got "?>" for , {"version"=>"1.0", "standalone"=>"yes"})

Also I am not sure where this is used other than testing so I am not clear where the issue was with xss attacks. Thoughts?

name.downcase!

unless closing
Expand Down
7 changes: 7 additions & 0 deletions actionpack/test/template/html-scanner/sanitizer_test.rb
Expand Up @@ -5,6 +5,13 @@ def setup
@sanitizer = nil # used by assert_sanitizer
end

def test_strip_tags_with_quote
sanitizer = HTML::FullSanitizer.new
string = '<" <img src="trollface.gif" onload="alert(1)"> hi'

assert_equal ' hi', sanitizer.sanitize(string)
end

def test_strip_tags
sanitizer = HTML::FullSanitizer.new
assert_equal("<<<bad html", sanitizer.sanitize("<<<bad html"))
Expand Down

0 comments on commit 586a944

Please sign in to comment.