Commit faafc74f authored by jplang's avatar jplang

Filter arbitrary class names and ids in rendered HTML output (#25503).

* Disallow setting arbitrary classes and ids via Textile syntax
* Only allow valid/supported languages for syntax highlighted code blocks

Patch by Jan Schulz-Hofen.

git-svn-id: https://svn.redmine.org/redmine/trunk@16502 e93f8b46-1217-0410-a6f0-8f06a7374b81
parent 8b834379
......@@ -35,7 +35,7 @@ module Redmine
end
def block_code(code, language)
if language.present?
if language.present? && Redmine::SyntaxHighlighting.language_supported?(language)
"<pre><code class=\"#{CGI.escapeHTML language} syntaxhl\">" +
Redmine::SyntaxHighlighting.highlight_by_language(code, language) +
"</code></pre>"
......
......@@ -121,8 +121,14 @@ module Redmine
text.gsub!(/<redpre#(\d+)>/) do
content = @pre_list[$1.to_i]
if content.match(/<code\s+class="(\w+)">\s?(.+)/m)
content = "<code class=\"#{$1} syntaxhl\">" +
Redmine::SyntaxHighlighting.highlight_by_language($2, $1)
language = $1
text = $2
if Redmine::SyntaxHighlighting.language_supported?(language)
content = "<code class=\"#{language} syntaxhl\">" +
Redmine::SyntaxHighlighting.highlight_by_language(text, language)
else
content = "<code>#{ERB::Util.h(text)}"
end
end
content
end
......
......@@ -494,7 +494,15 @@ class RedCloth3 < String
style << "text-align:#{ h_align( $& ) };" if text =~ A_HLGN
cls, id = $1, $2 if cls =~ /^(.*?)#(.*)$/
# add wiki-class- and wiki-id- to classes and ids to prevent setting of
# arbitrary classes and ids
cls = cls.split(/\s+/).map do |c|
c.starts_with?('wiki-class-') ? c : "wiki-class-#{c}"
end.join(' ') if cls
id = id.starts_with?('wiki-id-') ? id : "wiki-id-#{id}" if id
atts = ''
atts << " style=\"#{ style.join }\"" unless style.empty?
atts << " class=\"#{ cls }\"" unless cls.to_s.empty?
......@@ -1097,7 +1105,7 @@ class RedCloth3 < String
first.match(/<#{ OFFTAGS }([^>]*)>/)
tag = $1
$2.to_s.match(/(class\=("[^"]+"|'[^']+'))/i)
tag << " #{$1}" if $1
tag << " #{$1}" if $1 && tag == 'code'
@pre_list << "<#{ tag }>#{ aftertag }"
end
elsif $1 and codepre > 0
......
......@@ -117,7 +117,8 @@ class ApplicationHelperTest < Redmine::HelperTest
to_test = {
'!http://foo.bar/image.jpg!' => '<img src="http://foo.bar/image.jpg" alt="" />',
'floating !>http://foo.bar/image.jpg!' => 'floating <span style="float:right"><img src="http://foo.bar/image.jpg" alt="" /></span>',
'with class !(some-class)http://foo.bar/image.jpg!' => 'with class <img src="http://foo.bar/image.jpg" class="some-class" alt="" />',
'with class !(some-class)http://foo.bar/image.jpg!' => 'with class <img src="http://foo.bar/image.jpg" class="wiki-class-some-class" alt="" />',
'with class !(wiki-class-foo)http://foo.bar/image.jpg!' => 'with class <img src="http://foo.bar/image.jpg" class="wiki-class-foo" alt="" />',
'with style !{width:100px;height:100px}http://foo.bar/image.jpg!' => 'with style <img src="http://foo.bar/image.jpg" style="width:100px;height:100px;" alt="" />',
'with title !http://foo.bar/image.jpg(This is a title)!' => 'with title <img src="http://foo.bar/image.jpg" title="This is a title" alt="This is a title" />',
'with title !http://foo.bar/image.jpg(This is a double-quoted "title")!' => 'with title <img src="http://foo.bar/image.jpg" title="This is a double-quoted &quot;title&quot;" alt="This is a double-quoted &quot;title&quot;" />',
......@@ -911,11 +912,11 @@ RAW
"<pre><div>content</div></pre>" => "<pre>&lt;div&gt;content&lt;/div&gt;</pre>",
"HTML comment: <!-- no comments -->" => "<p>HTML comment: &lt;!-- no comments --&gt;</p>",
"<!-- opening comment" => "<p>&lt;!-- opening comment</p>",
# remove attributes except class
"<pre class='foo'>some text</pre>" => "<pre class='foo'>some text</pre>",
'<pre class="foo">some text</pre>' => '<pre class="foo">some text</pre>',
"<pre class='foo bar'>some text</pre>" => "<pre class='foo bar'>some text</pre>",
'<pre class="foo bar">some text</pre>' => '<pre class="foo bar">some text</pre>',
# remove attributes including class
"<pre class='foo'>some text</pre>" => "<pre>some text</pre>",
'<pre class="foo">some text</pre>' => '<pre>some text</pre>',
"<pre class='foo bar'>some text</pre>" => "<pre>some text</pre>",
'<pre class="foo bar">some text</pre>' => '<pre>some text</pre>',
"<pre onmouseover='alert(1)'>some text</pre>" => "<pre>some text</pre>",
# xss
'<pre><code class=""onmouseover="alert(1)">text</code></pre>' => '<pre><code>text</code></pre>',
......
......@@ -70,6 +70,15 @@ STR
end
end
def test_should_not_allow_invalid_language_for_code_blocks
text = <<-STR
~~~foo
test
~~~
STR
assert_equal "<pre>test\n</pre>", @formatter.new(text).to_html
end
def test_external_links_should_have_external_css_class
text = 'This is a [link](http://example.net/)'
assert_equal '<p>This is a <a href="http://example.net/" class="external">link</a></p>', @formatter.new(text).to_html.strip
......
......@@ -44,9 +44,7 @@ class Redmine::WikiFormatting::TextileFormatterTest < ActionView::TestCase
'*two*words*' => '<strong>two*words</strong>',
'*two * words*' => '<strong>two * words</strong>',
'*two* *words*' => '<strong>two</strong> <strong>words</strong>',
'*(two)* *(words)*' => '<strong>(two)</strong> <strong>(words)</strong>',
# with class
'*(foo)two words*' => '<strong class="foo">two words</strong>'
'*(two)* *(words)*' => '<strong>(two)</strong> <strong>(words)</strong>'
)
end
......@@ -535,6 +533,50 @@ STR
assert_match /\Ah1.\tHeading 1\s+Content 1\z/, @formatter.new(text).get_section(1).first
end
def test_should_not_allow_arbitrary_class_attribute_on_offtags
%w(code pre kbd).each do |tag|
assert_html_output({"<#{tag} class=\"foo\">test</#{tag}>" => "<#{tag}>test</#{tag}>"}, false)
end
assert_html_output({"<notextile class=\"foo\">test</notextile>" => "test"}, false)
end
def test_should_allow_valid_language_class_attribute_on_code_tags
assert_html_output({"<code class=\"ruby\">test</code>" => "<code class=\"ruby syntaxhl\"><span class=\"CodeRay\">test</span></code>"}, false)
end
def test_should_not_allow_valid_language_class_attribute_on_non_code_offtags
%w(pre kbd).each do |tag|
assert_html_output({"<#{tag} class=\"ruby\">test</#{tag}>" => "<#{tag}>test</#{tag}>"}, false)
end
assert_html_output({"<notextile class=\"ruby\">test</notextile>" => "test"}, false)
end
def test_should_prefix_class_attribute_on_tags
assert_html_output({
'!(foo)test.png!' => "<p><img src=\"test.png\" class=\"wiki-class-foo\" alt=\"\" /></p>",
'%(foo)test%' => "<p><span class=\"wiki-class-foo\">test</span></p>",
'p(foo). test' => "<p class=\"wiki-class-foo\">test</p>",
'|(foo). test|' => "<table>\n\t\t<tr>\n\t\t\t<td class=\"wiki-class-foo\">test</td>\n\t\t</tr>\n\t</table>",
}, false)
end
def test_should_prefix_id_attribute_on_tags
assert_html_output({
'!(#foo)test.png!' => "<p><img src=\"test.png\" id=\"wiki-id-foo\" alt=\"\" /></p>",
'%(#foo)test%' => "<p><span id=\"wiki-id-foo\">test</span></p>",
'p(#foo). test' => "<p id=\"wiki-id-foo\">test</p>",
'|(#foo). test|' => "<table>\n\t\t<tr>\n\t\t\t<td id=\"wiki-id-foo\">test</td>\n\t\t</tr>\n\t</table>",
}, false)
end
def test_should_not_prefix_class_and_id_attributes_already_prefixed
assert_html_output({
'!(wiki-class-foo#wiki-id-bar)test.png!' => "<p><img src=\"test.png\" class=\"wiki-class-foo\" id=\"wiki-id-bar\" alt=\"\" /></p>",
}, false)
end
private
def assert_html_output(to_test, expect_paragraph = true)
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment