Skip to content

Commit

Permalink
Migrate to SafeListSanitizer
Browse files Browse the repository at this point in the history
SafeList is easier to understand
  • Loading branch information
Juanito Fatas committed Apr 3, 2019
1 parent 5b73a09 commit 41b0b49
Show file tree
Hide file tree
Showing 4 changed files with 77 additions and 66 deletions.
20 changes: 10 additions & 10 deletions README.md
Expand Up @@ -41,22 +41,22 @@ link_sanitizer.sanitize('<a href="example.com">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
Expand Down
12 changes: 9 additions & 3 deletions lib/rails-html-sanitizer.rb
Expand Up @@ -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
Expand All @@ -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.
Expand All @@ -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,
Expand Down
39 changes: 22 additions & 17 deletions lib/rails/html/sanitizer.rb
Expand Up @@ -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.
Expand All @@ -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
Expand Down Expand Up @@ -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
72 changes: 36 additions & 36 deletions test/sanitizer_test.rb
Expand Up @@ -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 '&lt;script&gt;alert("XSS");&lt;/script&gt;', sanitizer.sanitize('<script><script></script>alert("XSS");<script><</script>/</script><script>script></script>', tags: %w(em))
end

def test_sanitize_nested_script_in_style
sanitizer = Rails::Html::WhiteListSanitizer.new
sanitizer = Rails::Html::SafeListSanitizer.new
assert_equal '&lt;script&gt;alert("XSS");&lt;/script&gt;', sanitizer.sanitize('<style><script></style>alert("XSS");<style><</style>/</style><style>script></style>', tags: %w(em))
end

Expand Down Expand Up @@ -255,38 +255,38 @@ def test_custom_attributes_overrides_allowed_attributes

def test_should_allow_custom_tags
text = "<u>foo</u>"
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 = "<u>foo</u> with <i>bar</i>"
assert_equal "<u>foo</u> with bar", white_list_sanitize(text, tags: %w(u))
assert_equal "<u>foo</u> with bar", safe_list_sanitize(text, tags: %w(u))
end

def test_should_allow_custom_tags_with_attributes
text = %(<blockquote cite="http://example.com/">foo</blockquote>)
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 = %(<blockquote foo="bar">Lorem ipsum</blockquote>)
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 = '<p style="color: #000; background-image: url(http://www.ragingplatypus.com/i/cam-full.jpg);"></p>'
assert_equal '<p style="color: #000;"></p>', white_list_sanitize(input, attributes: %w(style))
assert_equal '<p style="color: #000;"></p>', 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('<a>some html</a>', tags: 'foo')
safe_list_sanitize('<a>some html</a>', tags: 'foo')
end
end

def test_should_raise_argument_error_if_attributes_is_not_enumerable
assert_raises ArgumentError do
white_list_sanitize('<a>some html</a>', attributes: 'foo')
safe_list_sanitize('<a>some html</a>', attributes: 'foo')
end
end

Expand All @@ -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('<a>some html</a>', scrubber: scrubber)
safe_list_sanitize('<a>some html</a>', scrubber: scrubber)
end
end

Expand All @@ -304,19 +304,19 @@ def test_should_accept_loofah_inheriting_scrubber
def scrubber.scrub(node); node.name = 'h1'; end

html = "<script>hello!</script>"
assert_equal "<h1>hello!</h1>", white_list_sanitize(html, scrubber: scrubber)
assert_equal "<h1>hello!</h1>", 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 = "<script>hello!</script>"
assert_equal "<h1>hello!</h1>", white_list_sanitize(html, scrubber: scrubber)
assert_equal "<h1>hello!</h1>", 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 = "<script>hello!</script>"
assert_equal "<h1>hello!</h1>", white_list_sanitize(html, scrubber: scrubber, tags: ['foo'])
assert_equal "<h1>hello!</h1>", safe_list_sanitize(html, scrubber: scrubber, tags: ['foo'])
end

[%w(img src), %w(a href)].each do |(tag, attr)|
Expand Down Expand Up @@ -468,7 +468,7 @@ def test_x03a_legitimate
end

def test_sanitize_ascii_8bit_string
white_list_sanitize('<a>hello</a>'.encode('ASCII-8BIT')).tap do |sanitized|
safe_list_sanitize('<a>hello</a>'.encode('ASCII-8BIT')).tap do |sanitized|
assert_equal '<a>hello</a>', sanitized
assert_equal Encoding::UTF_8, sanitized.encoding
end
Expand All @@ -481,45 +481,45 @@ def test_sanitize_data_attributes

def test_allow_data_attribute_if_requested
text = %(<a data-foo="foo">foo</a>)
assert_equal %(<a data-foo="foo">foo</a>), white_list_sanitize(text, attributes: ['data-foo'])
assert_equal %(<a data-foo="foo">foo</a>), 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
skip if RUBY_VERSION < "2.3"

html = %{<a href='examp<!--" unsafeattr=foo()>-->le.com'>test</a>}

text = white_list_sanitize(html)
text = safe_list_sanitize(html)

assert_equal %{<a href=\"examp&lt;!--%22%20unsafeattr=foo()&gt;--&gt;le.com\">test</a>}, 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
skip if RUBY_VERSION < "2.3"

html = %{<a src='examp<!--" unsafeattr=foo()>-->le.com'>test</a>}

text = white_list_sanitize(html)
text = safe_list_sanitize(html)

assert_equal %{<a src=\"examp&lt;!--%22%20unsafeattr=foo()&gt;--&gt;le.com\">test</a>}, 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
skip if RUBY_VERSION < "2.3"

html = %{<a name='examp<!--" unsafeattr=foo()>-->le.com'>test</a>}

text = white_list_sanitize(html)
text = safe_list_sanitize(html)

assert_equal %{<a name=\"examp&lt;!--%22%20unsafeattr=foo()&gt;--&gt;le.com\">test</a>}, 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
skip if RUBY_VERSION < "2.3"

html = %{<a action='examp<!--" unsafeattr=foo()>-->le.com'>test</a>}

text = white_list_sanitize(html, attributes: ['action'])
text = safe_list_sanitize(html, attributes: ['action'])

assert_equal %{<a action=\"examp&lt;!--%22%20unsafeattr=foo()&gt;--&gt;le.com\">test</a>}, text
end
Expand All @@ -538,35 +538,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

0 comments on commit 41b0b49

Please sign in to comment.