Skip to content

Commit

Permalink
Fix XSS vulnerability on table of content generation
Browse files Browse the repository at this point in the history
  • Loading branch information
lulalala authored and gjtorikian committed Jun 26, 2018
1 parent 2b34ff9 commit e132fd5
Show file tree
Hide file tree
Showing 3 changed files with 11 additions and 2 deletions.
1 change: 1 addition & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -186,6 +186,7 @@ gem 'rouge'
* `PlainTextInputFilter` - `escape_utils`
* `SanitizationFilter` - `sanitize`
* `SyntaxHighlightFilter` - `rouge`
* `TableOfContentsFilter` - `escape_utils`
* `TextileFilter` - `RedCloth`

_Note:_ See [Gemfile](/Gemfile) `:test` block for version requirements.
Expand Down
4 changes: 3 additions & 1 deletion lib/html/pipeline/toc_filter.rb
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
HTML::Pipeline.require_dependency('escape_utils', 'TableOfContentsFilter')

module HTML
class Pipeline
# HTML filter that adds an 'id' attribute to all headers
Expand Down Expand Up @@ -43,7 +45,7 @@ def call
uniq = headers[id] > 0 ? "-#{headers[id]}" : ''
headers[id] += 1
if header_content = node.children.first
result[:toc] << %(<li><a href="##{id}#{uniq}">#{text}</a></li>\n)
result[:toc] << %(<li><a href="##{id}#{uniq}">#{EscapeUtils.escape_html(text)}</a></li>\n)
header_content.add_previous_sibling(%(<a id="#{id}#{uniq}" class="anchor" href="##{id}#{uniq}" aria-hidden="true">#{anchor_icon}</a>))
end
end
Expand Down
8 changes: 7 additions & 1 deletion test/html/pipeline/toc_filter_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,12 @@ def test_all_header_tags_are_found_when_adding_anchors
assert_equal 6, doc.search('a').size
end

def test_toc_outputs_escaped_html
@orig = %(<h1>&lt;img src="x" onerror="alert(42)"&gt;</h1>)

refute_includes toc, %(<img src="x" onerror="alert(42)">)
end

def test_toc_is_complete
@orig = %(<h1>"Funky President" by James Brown</h1>
<h2>"It's My Thing" by Marva Whitney</h2>
Expand All @@ -101,7 +107,7 @@ def test_toc_is_complete
<h6>"Ruthless Villain" by Eazy-E</h6>
<h7>"Be Thankful for What You Got" by William DeVaughn</h7>)

expected = %(<ul class="section-nav">\n<li><a href="#funky-president-by-james-brown">"Funky President" by James Brown</a></li>\n<li><a href="#its-my-thing-by-marva-whitney">"It's My Thing" by Marva Whitney</a></li>\n<li><a href="#boogie-back-by-roy-ayers">"Boogie Back" by Roy Ayers</a></li>\n<li><a href="#feel-good-by-fancy">"Feel Good" by Fancy</a></li>\n<li><a href="#funky-drummer-by-james-brown">"Funky Drummer" by James Brown</a></li>\n<li><a href="#ruthless-villain-by-eazy-e">"Ruthless Villain" by Eazy-E</a></li>\n</ul>)
expected = %(<ul class="section-nav">\n<li><a href="#funky-president-by-james-brown">&quot;Funky President&quot; by James Brown</a></li>\n<li><a href="#its-my-thing-by-marva-whitney">&quot;It&#39;s My Thing&quot; by Marva Whitney</a></li>\n<li><a href="#boogie-back-by-roy-ayers">&quot;Boogie Back&quot; by Roy Ayers</a></li>\n<li><a href="#feel-good-by-fancy">&quot;Feel Good&quot; by Fancy</a></li>\n<li><a href="#funky-drummer-by-james-brown">&quot;Funky Drummer&quot; by James Brown</a></li>\n<li><a href="#ruthless-villain-by-eazy-e">&quot;Ruthless Villain&quot; by Eazy-E</a></li>\n</ul>)

assert_equal expected, toc
end
Expand Down

0 comments on commit e132fd5

Please sign in to comment.