Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Migrate to SafeListSanitizer #87

Merged
merged 1 commit into from
Apr 27, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 10 additions & 10 deletions README.md
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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."

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is missing a space, shows up as "isdeprecated" in deprecation messages.

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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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