Merge pull request #933 from nobu/h1-1187156-xss

Escape links

Author: nobu

lib/rdoc/generator/template/darkfish/index.rhtml

@@ -17,6 +17,6 @@
       main_page = @files.find { |f| f.full_name == @options.main_page } then %>
 <%= main_page.description %>
 <%- else -%>
-<p>This is the API documentation for <%= @title %>.
+<p>This is the API documentation for <%= h @title %>.
 <%- end -%>
 </main>

lib/rdoc/markup/to_html.rb

@@ -84,7 +84,7 @@ def init_link_notation_regexp_handlings
   def handle_RDOCLINK url # :nodoc:
     case url
     when /^rdoc-ref:/
-      $'
+      CGI.escapeHTML($')
     when /^rdoc-label:/
       text = $'
 
@@ -95,13 +95,11 @@ def handle_RDOCLINK url # :nodoc:
              else                    text
              end
 
-      gen_url url, text
+      gen_url CGI.escapeHTML(url), CGI.escapeHTML(text)
     when /^rdoc-image:/
-      "<img src=\"#{$'}\">"
-    else
-      url =~ /\Ardoc-[a-z]+:/
-
-      $'
+      %[<img src=\"#{CGI.escapeHTML($')}\">]
+    when /\Ardoc-[a-z]+:/
+      CGI.escapeHTML($')
     end
   end
 
@@ -125,7 +123,7 @@ def handle_regexp_HARD_BREAK target
   #   Reference to a local file relative to the output directory.
 
   def handle_regexp_HYPERLINK(target)
-    url = target.text
+    url = CGI.escapeHTML(target.text)
 
     gen_url url, url
   end
@@ -154,9 +152,13 @@ def handle_regexp_TIDYLINK(target)
       text =~ /^\{(.*)\}\[(.*?)\]$/ or text =~ /^(\S+)\[(.*?)\]$/
 
     label = $1
-    url   = $2
+    url   = CGI.escapeHTML($2)
 
-    label = handle_RDOCLINK label if /^rdoc-image:/ =~ label
+    if /^rdoc-image:/ =~ label
+      label = handle_RDOCLINK(label)
+    else
+      label = CGI.escapeHTML(label)
+    end
 
     gen_url url, label
   end

test/rdoc/test_rdoc_generator_darkfish.rb

@@ -248,6 +248,22 @@ def test_template_stylesheets
     assert_include File.read('index.html'), %Q[href="./#{base}"]
   end
 
+  def test_title
+    title = "RDoc Test".freeze
+    @options.title = title
+    @g.generate
+
+    assert_main_title(File.read('index.html'), title)
+  end
+
+  def test_title_escape
+    title = %[<script>alert("RDoc")</script>].freeze
+    @options.title = title
+    @g.generate
+
+    assert_main_title(File.read('index.html'), title)
+  end
+
   ##
   # Asserts that +filename+ has a link count greater than 1 if hard links to
   # @tmpdir are supported.
@@ -271,4 +287,9 @@ def assert_hard_link filename
                     "#{filename} is not hard-linked"
   end
 
+  def assert_main_title(content, title)
+    title = CGI.escapeHTML(title)
+    assert_equal(title, content[%r[<title>(.*?)<\/title>]im, 1])
+    assert_include(content[%r[<main\s[^<>]*+>\s*(.*?)</main>]im, 1], title)
+  end
 end

test/rdoc/test_rdoc_markup_to_html.rb

@@ -665,6 +665,26 @@ def test_convert_RDOCLINK_ref
     assert_equal "\n<p>C</p>\n", result
   end
 
+  def test_convert_RDOCLINK_escape_image
+    assert_escaped '<script>', 'rdoc-image:"><script>alert(`rdoc-image`)</script>"'
+  end
+
+  def test_convert_RDOCLINK_escape_label_id
+    assert_escaped '<script>', 'rdoc-label::path::"><script>alert(`rdoc-label_id`)</script>"'
+  end
+
+  def test_convert_RDOCLINK_escape_label_path
+    assert_escaped '<script>', 'rdoc-label::"><script>alert(`rdoc-label_path`)</script>"'
+  end
+
+  def test_convert_RDOCLINK_escape_ref
+    assert_escaped '<script>', 'rdoc-ref:"><script>alert(`rdoc-ref`)</script>"'
+  end
+
+  def test_convert_RDOCLINK_escape_xxx
+    assert_escaped '<script>', 'rdoc-xxx:"><script>alert(`rdoc-xxx`)</script>"'
+  end
+
   def test_convert_TIDYLINK_footnote
     result = @to.convert 'text{*1}[rdoc-label:foottext-1:footmark-1]'
 
@@ -690,6 +710,11 @@ def test_convert_TIDYLINK_image
       "\n<p><a href=\"http://example.com\"><img src=\"path/to/image.jpg\"></a></p>\n"
 
     assert_equal expected, result
+
+    result =
+      @to.convert '{rdoc-image:<script>alert`link text`</script>}[http://example.com]'
+
+    assert_not_include result, "<script>"
   end
 
   def test_convert_TIDYLINK_rdoc_label
@@ -704,6 +729,23 @@ def test_convert_TIDYLINK_irc
     assert_equal "\n<p><a href=\"irc://irc.freenode.net/#ruby-lang\">ruby-lang</a></p>\n", result
   end
 
+  def test_convert_TIDYLINK_escape_text
+    assert_escaped '<script>', '{<script>alert`link text`</script>}[a]'
+    assert_escaped '<script>', 'x:/<script>alert(1);</script>[[]'
+  end
+
+  def test_convert_TIDYLINK_escape_javascript
+    assert_not_include '{click}[javascript:alert`javascript_scheme`]', '<a href="javascript:'
+  end
+
+  def test_convert_TIDYLINK_escape_onmouseover
+    assert_escaped '"/onmouseover="', '{onmouseover}[http://"/onmouseover="alert`on_mouse_link`"]'
+  end
+
+  def test_convert_TIDYLINK_escape_onerror
+    assert_escaped '"onerror="', '{link_image}[http://"onerror="alert`link_image`".png]'
+  end
+
   def test_convert_with_exclude_tag
     assert_equal "\n<p><code>aaa</code>[:symbol]</p>\n", @to.convert('+aaa+[:symbol]')
     assert_equal "\n<p><code>aaa[:symbol]</code></p>\n", @to.convert('+aaa[:symbol]+')
@@ -794,6 +836,11 @@ def test_handle_regexp_HYPERLINK_irc
     assert_equal '<a href="irc://irc.freenode.net/#ruby-lang">irc.freenode.net/#ruby-lang</a>', link
   end
 
+  def test_handle_regexp_HYPERLINK_escape
+    code = 'irc://irc.freenode.net/"><script>alert(`irc`)</script><a"'
+    assert_escaped '<script>', code
+  end
+
   def test_list_verbatim_2
     str = "* one\n    verb1\n    verb2\n* two\n"
 
@@ -903,5 +950,11 @@ def test_accept_table
     assert_include(res[%r<<td[^<>]*>.*em.*</td>>], '<em>em</em>')
     assert_include(res[%r<<td[^<>]*>.*strong.*</td>>], '<strong>strong</strong>')
   end
+
+  def assert_escaped(unexpected, code)
+    result = @to.convert(code)
+    assert_not_include result, unexpected
+    assert_include result, CGI.escapeHTML(unexpected)
+  end
 end