From 911899f569e8e906ab1ee1c5be00dece1bec2431 Mon Sep 17 00:00:00 2001 From: Juanito Fatas Date: Thu, 21 Mar 2019 10:05:14 +0900 Subject: [PATCH] Migrate to SafeListSanitizer SafeList is easier to understand --- README.md | 20 +++++------ lib/rails-html-sanitizer.rb | 12 +++++-- lib/rails/html/sanitizer.rb | 39 +++++++++++--------- test/sanitizer_test.rb | 72 ++++++++++++++++++------------------- 4 files changed, 77 insertions(+), 66 deletions(-) diff --git a/README.md b/README.md index ed01f05..b3e9fb0 100644 --- a/README.md +++ b/README.md @@ -41,22 +41,22 @@ link_sanitizer.sanitize('Only the link text will be kept.< # => Only the link text will be kept. ``` -#### WhiteListSanitizer +#### SafeListSanitizer ```ruby -white_list_sanitizer = Rails::Html::WhiteListSanitizer.new +safe_list_sanitizer = Rails::Html::SafeListSanitizer.new -# sanitize via an extensive white list of allowed elements -white_list_sanitizer.sanitize(@article.body) +# sanitize via an extensive safe list of allowed elements +safe_list_sanitizer.sanitize(@article.body) -# white list only the supplied tags and attributes -white_list_sanitizer.sanitize(@article.body, tags: %w(table tr td), attributes: %w(id class style)) +# safe list only the supplied tags and attributes +safe_list_sanitizer.sanitize(@article.body, tags: %w(table tr td), attributes: %w(id class style)) -# white list via a custom scrubber -white_list_sanitizer.sanitize(@article.body, scrubber: ArticleScrubber.new) +# safe list via a custom scrubber +safe_list_sanitizer.sanitize(@article.body, scrubber: ArticleScrubber.new) -# white list sanitizer can also sanitize css -white_list_sanitizer.sanitize_css('background-color: #000;') +# safe list sanitizer can also sanitize css +safe_list_sanitizer.sanitize_css('background-color: #000;') ``` ### Scrubbers diff --git a/lib/rails-html-sanitizer.rb b/lib/rails-html-sanitizer.rb index 494a56f..f8d1736 100644 --- a/lib/rails-html-sanitizer.rb +++ b/lib/rails-html-sanitizer.rb @@ -15,8 +15,14 @@ def link_sanitizer Html::LinkSanitizer end + def safe_list_sanitizer + Html::SafeListSanitizer + end + def white_list_sanitizer - Html::WhiteListSanitizer + ActiveSupport::Deprecation.warn "warning: white_list_sanitizer is" \ + "deprecated, please use safe_list_sanitizer instead." + safe_list_sanitizer end end end @@ -34,7 +40,7 @@ module ClassMethods # end # def sanitized_allowed_tags=(tags) - sanitizer_vendor.white_list_sanitizer.allowed_tags = tags + sanitizer_vendor.safe_list_sanitizer.allowed_tags = tags end # Replaces the allowed HTML attributes for the +sanitize+ helper. @@ -44,7 +50,7 @@ def sanitized_allowed_tags=(tags) # end # def sanitized_allowed_attributes=(attributes) - sanitizer_vendor.white_list_sanitizer.allowed_attributes = attributes + sanitizer_vendor.safe_list_sanitizer.allowed_attributes = attributes end [:protocol_separator, diff --git a/lib/rails/html/sanitizer.rb b/lib/rails/html/sanitizer.rb index 7fc6994..f9bef9f 100644 --- a/lib/rails/html/sanitizer.rb +++ b/lib/rails/html/sanitizer.rb @@ -57,8 +57,8 @@ def sanitize(html, options = {}) end end - # === Rails::Html::WhiteListSanitizer - # Sanitizes html and css from an extensive white list (see link further down). + # === Rails::Html::SafeListSanitizer + # Sanitizes html and css from an extensive safe list (see link further down). # # === Whitespace # We can't make any guarantees about whitespace being kept or stripped. @@ -72,34 +72,34 @@ def sanitize(html, options = {}) # so automatically. # # === Options - # Sanitizes both html and css via the white lists found here: + # Sanitizes both html and css via the safe lists found here: # https://github.com/flavorjones/loofah/blob/master/lib/loofah/html5/whitelist.rb # - # WhiteListSanitizer also accepts options to configure - # the white list used when sanitizing html. + # SafeListSanitizer also accepts options to configure + # the safe list used when sanitizing html. # There's a class level option: - # Rails::Html::WhiteListSanitizer.allowed_tags = %w(table tr td) - # Rails::Html::WhiteListSanitizer.allowed_attributes = %w(id class style) + # Rails::Html::SafeListSanitizer.allowed_tags = %w(table tr td) + # Rails::Html::SafeListSanitizer.allowed_attributes = %w(id class style) # # Tags and attributes can also be passed to +sanitize+. # Passed options take precedence over the class level options. # # === Examples - # white_list_sanitizer = Rails::Html::WhiteListSanitizer.new + # safe_list_sanitizer = Rails::Html::SafeListSanitizer.new # # Sanitize css doesn't take options - # white_list_sanitizer.sanitize_css('background-color: #000;') + # safe_list_sanitizer.sanitize_css('background-color: #000;') # - # Default: sanitize via a extensive white list of allowed elements - # white_list_sanitizer.sanitize(@article.body) + # Default: sanitize via a extensive safe list of allowed elements + # safe_list_sanitizer.sanitize(@article.body) # - # White list via the supplied tags and attributes - # white_list_sanitizer.sanitize(@article.body, tags: %w(table tr td), + # Safe list via the supplied tags and attributes + # safe_list_sanitizer.sanitize(@article.body, tags: %w(table tr td), # attributes: %w(id class style)) # - # White list via a custom scrubber - # white_list_sanitizer.sanitize(@article.body, scrubber: ArticleScrubber.new) - class WhiteListSanitizer < Sanitizer + # Safe list via a custom scrubber + # safe_list_sanitizer.sanitize(@article.body, scrubber: ArticleScrubber.new) + class SafeListSanitizer < Sanitizer class << self attr_accessor :allowed_tags attr_accessor :allowed_attributes @@ -146,7 +146,12 @@ def allowed_tags(options) def allowed_attributes(options) options[:attributes] || self.class.allowed_attributes - end + end + end + + WhiteListSanitizer = SafeListSanitizer + if Object.respond_to?(:deprecate_constant) + deprecate_constant :WhiteListSanitizer end end end diff --git a/test/sanitizer_test.rb b/test/sanitizer_test.rb index 7bcab6f..4369ab0 100644 --- a/test/sanitizer_test.rb +++ b/test/sanitizer_test.rb @@ -12,12 +12,12 @@ def test_sanitizer_sanitize_raises_not_implemented_error end def test_sanitize_nested_script - sanitizer = Rails::Html::WhiteListSanitizer.new + sanitizer = Rails::Html::SafeListSanitizer.new assert_equal '<script>alert("XSS");</script>', sanitizer.sanitize('alert("XSS");/', tags: %w(em)) end def test_sanitize_nested_script_in_style - sanitizer = Rails::Html::WhiteListSanitizer.new + sanitizer = Rails::Html::SafeListSanitizer.new assert_equal '<script>alert("XSS");</script>', sanitizer.sanitize('alert("XSS");/', tags: %w(em)) end @@ -255,38 +255,38 @@ def test_custom_attributes_overrides_allowed_attributes def test_should_allow_custom_tags text = "foo" - assert_equal text, white_list_sanitize(text, tags: %w(u)) + assert_equal text, safe_list_sanitize(text, tags: %w(u)) end def test_should_allow_only_custom_tags text = "foo with bar" - assert_equal "foo with bar", white_list_sanitize(text, tags: %w(u)) + assert_equal "foo with bar", safe_list_sanitize(text, tags: %w(u)) end def test_should_allow_custom_tags_with_attributes text = %(
foo
) - assert_equal text, white_list_sanitize(text) + assert_equal text, safe_list_sanitize(text) end def test_should_allow_custom_tags_with_custom_attributes text = %(
Lorem ipsum
) - assert_equal text, white_list_sanitize(text, attributes: ['foo']) + assert_equal text, safe_list_sanitize(text, attributes: ['foo']) end def test_scrub_style_if_style_attribute_option_is_passed input = '

' - assert_equal '

', white_list_sanitize(input, attributes: %w(style)) + assert_equal '

', safe_list_sanitize(input, attributes: %w(style)) end def test_should_raise_argument_error_if_tags_is_not_enumerable assert_raises ArgumentError do - white_list_sanitize('
some html', tags: 'foo') + safe_list_sanitize('some html', tags: 'foo') end end def test_should_raise_argument_error_if_attributes_is_not_enumerable assert_raises ArgumentError do - white_list_sanitize('some html', attributes: 'foo') + safe_list_sanitize('some html', attributes: 'foo') end end @@ -295,7 +295,7 @@ def test_should_not_accept_non_loofah_inheriting_scrubber def scrubber.scrub(node); node.name = 'h1'; end assert_raises Loofah::ScrubberNotFound do - white_list_sanitize('some html', scrubber: scrubber) + safe_list_sanitize('some html', scrubber: scrubber) end end @@ -304,19 +304,19 @@ def test_should_accept_loofah_inheriting_scrubber def scrubber.scrub(node); node.name = 'h1'; end html = "" - assert_equal "

hello!

", white_list_sanitize(html, scrubber: scrubber) + assert_equal "

hello!

", safe_list_sanitize(html, scrubber: scrubber) end def test_should_accept_loofah_scrubber_that_wraps_a_block scrubber = Loofah::Scrubber.new { |node| node.name = 'h1' } html = "" - assert_equal "

hello!

", white_list_sanitize(html, scrubber: scrubber) + assert_equal "

hello!

", safe_list_sanitize(html, scrubber: scrubber) end def test_custom_scrubber_takes_precedence_over_other_options scrubber = Loofah::Scrubber.new { |node| node.name = 'h1' } html = "" - assert_equal "

hello!

", white_list_sanitize(html, scrubber: scrubber, tags: ['foo']) + assert_equal "

hello!

", safe_list_sanitize(html, scrubber: scrubber, tags: ['foo']) end [%w(img src), %w(a href)].each do |(tag, attr)| @@ -468,7 +468,7 @@ def test_x03a_legitimate end def test_sanitize_ascii_8bit_string - white_list_sanitize('hello'.encode('ASCII-8BIT')).tap do |sanitized| + safe_list_sanitize('hello'.encode('ASCII-8BIT')).tap do |sanitized| assert_equal 'hello', sanitized assert_equal Encoding::UTF_8, sanitized.encoding end @@ -481,37 +481,37 @@ def test_sanitize_data_attributes def test_allow_data_attribute_if_requested text = %(foo) - assert_equal %(foo), white_list_sanitize(text, attributes: ['data-foo']) + assert_equal %(foo), safe_list_sanitize(text, attributes: ['data-foo']) end - def test_uri_escaping_of_href_attr_in_a_tag_in_white_list_sanitizer + def test_uri_escaping_of_href_attr_in_a_tag_in_safe_list_sanitizer html = %{test} - text = white_list_sanitize(html) + text = safe_list_sanitize(html) assert_equal %{test}, text end - def test_uri_escaping_of_src_attr_in_a_tag_in_white_list_sanitizer + def test_uri_escaping_of_src_attr_in_a_tag_in_safe_list_sanitizer html = %{test} - text = white_list_sanitize(html) + text = safe_list_sanitize(html) assert_equal %{test}, text end - def test_uri_escaping_of_name_attr_in_a_tag_in_white_list_sanitizer + def test_uri_escaping_of_name_attr_in_a_tag_in_safe_list_sanitizer html = %{test} - text = white_list_sanitize(html) + text = safe_list_sanitize(html) assert_equal %{test}, text end - def test_uri_escaping_of_name_action_in_a_tag_in_white_list_sanitizer + def test_uri_escaping_of_name_action_in_a_tag_in_safe_list_sanitizer html = %{test} - text = white_list_sanitize(html, attributes: ['action']) + text = safe_list_sanitize(html, attributes: ['action']) assert_equal %{test}, text end @@ -530,35 +530,35 @@ def link_sanitize(input, options = {}) Rails::Html::LinkSanitizer.new.sanitize(input, options) end - def white_list_sanitize(input, options = {}) - Rails::Html::WhiteListSanitizer.new.sanitize(input, options) + def safe_list_sanitize(input, options = {}) + Rails::Html::SafeListSanitizer.new.sanitize(input, options) end def assert_sanitized(input, expected = nil) if input - assert_dom_equal expected || input, white_list_sanitize(input) + assert_dom_equal expected || input, safe_list_sanitize(input) else - assert_nil white_list_sanitize(input) + assert_nil safe_list_sanitize(input) end end def sanitize_css(input) - Rails::Html::WhiteListSanitizer.new.sanitize_css(input) + Rails::Html::SafeListSanitizer.new.sanitize_css(input) end def scope_allowed_tags(tags) - old_tags = Rails::Html::WhiteListSanitizer.allowed_tags - Rails::Html::WhiteListSanitizer.allowed_tags = tags - yield Rails::Html::WhiteListSanitizer.new + old_tags = Rails::Html::SafeListSanitizer.allowed_tags + Rails::Html::SafeListSanitizer.allowed_tags = tags + yield Rails::Html::SafeListSanitizer.new ensure - Rails::Html::WhiteListSanitizer.allowed_tags = old_tags + Rails::Html::SafeListSanitizer.allowed_tags = old_tags end def scope_allowed_attributes(attributes) - old_attributes = Rails::Html::WhiteListSanitizer.allowed_attributes - Rails::Html::WhiteListSanitizer.allowed_attributes = attributes - yield Rails::Html::WhiteListSanitizer.new + old_attributes = Rails::Html::SafeListSanitizer.allowed_attributes + Rails::Html::SafeListSanitizer.allowed_attributes = attributes + yield Rails::Html::SafeListSanitizer.new ensure - Rails::Html::WhiteListSanitizer.allowed_attributes = old_attributes + Rails::Html::SafeListSanitizer.allowed_attributes = old_attributes end end