From 8be6913990c30f63618173da722148892348dcc9 Mon Sep 17 00:00:00 2001 From: Charlie Somerville Date: Wed, 13 Feb 2013 09:09:53 +1100 Subject: fix incorrect ^$ usage leading to XSS in sanitize_css [CVE-2013-1855] --- actionpack/lib/action_view/vendor/html-scanner/html/sanitizer.rb | 6 +++--- actionpack/test/template/html-scanner/sanitizer_test.rb | 5 +++++ 2 files changed, 8 insertions(+), 3 deletions(-) (limited to 'actionpack') diff --git a/actionpack/lib/action_view/vendor/html-scanner/html/sanitizer.rb b/actionpack/lib/action_view/vendor/html-scanner/html/sanitizer.rb index 6b4ececda2..6b8cb3acc7 100644 --- a/actionpack/lib/action_view/vendor/html-scanner/html/sanitizer.rb +++ b/actionpack/lib/action_view/vendor/html-scanner/html/sanitizer.rb @@ -121,8 +121,8 @@ module HTML style = style.to_s.gsub(/url\s*\(\s*[^\s)]+?\s*\)\s*/, ' ') # gauntlet - if style !~ /^([:,;#%.\sa-zA-Z0-9!]|\w-\w|\'[\s\w]+\'|\"[\s\w]+\"|\([\d,\s]+\))*$/ || - style !~ /^(\s*[-\w]+\s*:\s*[^:;]*(;|$)\s*)*$/ + if style !~ /\A([:,;#%.\sa-zA-Z0-9!]|\w-\w|\'[\s\w]+\'|\"[\s\w]+\"|\([\d,\s]+\))*\z/ || + style !~ /\A(\s*[-\w]+\s*:\s*[^:;]*(;|$)\s*)*\z/ return '' end @@ -133,7 +133,7 @@ module HTML elsif shorthand_css_properties.include?(prop.split('-')[0].downcase) unless val.split().any? do |keyword| !allowed_css_keywords.include?(keyword) && - keyword !~ /^(#[0-9a-f]+|rgb\(\d+%?,\d*%?,?\d*%?\)?|\d{0,2}\.?\d{0,2}(cm|em|ex|in|mm|pc|pt|px|%|,|\))?)$/ + keyword !~ /\A(#[0-9a-f]+|rgb\(\d+%?,\d*%?,?\d*%?\)?|\d{0,2}\.?\d{0,2}(cm|em|ex|in|mm|pc|pt|px|%|,|\))?)\z/ end clean << prop + ': ' + val + ';' end diff --git a/actionpack/test/template/html-scanner/sanitizer_test.rb b/actionpack/test/template/html-scanner/sanitizer_test.rb index d9b57776c9..65eb41e839 100644 --- a/actionpack/test/template/html-scanner/sanitizer_test.rb +++ b/actionpack/test/template/html-scanner/sanitizer_test.rb @@ -279,6 +279,11 @@ class SanitizerTest < ActionController::TestCase assert_equal '', sanitize_css(raw) end + def test_should_sanitize_across_newlines + raw = %(\nwidth:\nexpression(alert('XSS'));\n) + assert_equal '', sanitize_css(raw) + end + def test_should_sanitize_img_vbscript assert_sanitized %(), '' end -- cgit v1.2.3 From e115ace02a88290d2fc707b4979f23728c300950 Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Fri, 15 Mar 2013 15:04:00 -0700 Subject: fix protocol checking in sanitization [CVE-2013-1857] --- .../lib/action_view/vendor/html-scanner/html/sanitizer.rb | 4 ++-- actionpack/test/template/html-scanner/sanitizer_test.rb | 10 ++++++++++ 2 files changed, 12 insertions(+), 2 deletions(-) (limited to 'actionpack') diff --git a/actionpack/lib/action_view/vendor/html-scanner/html/sanitizer.rb b/actionpack/lib/action_view/vendor/html-scanner/html/sanitizer.rb index 6b8cb3acc7..30b6b8b141 100644 --- a/actionpack/lib/action_view/vendor/html-scanner/html/sanitizer.rb +++ b/actionpack/lib/action_view/vendor/html-scanner/html/sanitizer.rb @@ -77,7 +77,7 @@ module HTML # A regular expression of the valid characters used to separate protocols like # the ':' in 'http://foo.com' - self.protocol_separator = /:|(�*58)|(p)|(%|%)3A/ + self.protocol_separator = /:|(�*58)|(p)|(�*3a)|(%|%)3A/i # Specifies a Set of HTML attributes that can have URIs. self.uri_attributes = Set.new(%w(href src cite action longdesc xlink:href lowsrc)) @@ -182,7 +182,7 @@ module HTML def contains_bad_protocols?(attr_name, value) uri_attributes.include?(attr_name) && - (value =~ /(^[^\/:]*):|(�*58)|(p)|(%|%)3A/ && !allowed_protocols.include?(value.split(protocol_separator).first.downcase.strip)) + (value =~ /(^[^\/:]*):|(�*58)|(p)|(�*3a)|(%|%)3A/i && !allowed_protocols.include?(value.split(protocol_separator).first.downcase.strip)) end end end diff --git a/actionpack/test/template/html-scanner/sanitizer_test.rb b/actionpack/test/template/html-scanner/sanitizer_test.rb index 65eb41e839..b1c1b83807 100644 --- a/actionpack/test/template/html-scanner/sanitizer_test.rb +++ b/actionpack/test/template/html-scanner/sanitizer_test.rb @@ -200,6 +200,7 @@ class SanitizerTest < ActionController::TestCase %(), %(), %(), + %(), %()].each_with_index do |img_hack, i| define_method "test_should_not_fall_for_xss_image_hack_#{i+1}" do assert_sanitized img_hack, "" @@ -304,6 +305,15 @@ class SanitizerTest < ActionController::TestCase assert_sanitized "" end + def test_x03a + assert_sanitized %(), "" + assert_sanitized %(), "" + assert_sanitized %(), %() + assert_sanitized %(), "" + assert_sanitized %(), "" + assert_sanitized %(), %() + end + protected def assert_sanitized(input, expected = nil) @sanitizer ||= HTML::WhiteListSanitizer.new -- cgit v1.2.3