Commit 4511d5059cc9e0a85b4e2af07b7ebf16bae3712a
Exists in
master
and in
20 other branches
Merge branch 'fix-html-sanitizer'
Replaces merge request !729
Showing
3 changed files
with
7 additions
and
27 deletions
Show diff stats
config/initializers/sanitizer.rb
| ... | ... | @@ -12,24 +12,3 @@ Loofah::HTML5::WhiteList::ALLOWED_ATTRIBUTES.merge %w[ |
| 12 | 12 | style target codebase archive classid code flashvars scrolling frameborder controls autoplay colspan |
| 13 | 13 | ] |
| 14 | 14 | |
| 15 | -# do not escape COMMENT_NODE | |
| 16 | -require 'loofah/scrubber' | |
| 17 | -module Loofah | |
| 18 | - class Scrubber | |
| 19 | - private | |
| 20 | - | |
| 21 | - def html5lib_sanitize node | |
| 22 | - case node.type | |
| 23 | - when Nokogiri::XML::Node::ELEMENT_NODE | |
| 24 | - if HTML5::Scrub.allowed_element? node.name | |
| 25 | - HTML5::Scrub.scrub_attributes node | |
| 26 | - return Scrubber::CONTINUE | |
| 27 | - end | |
| 28 | - when Nokogiri::XML::Node::TEXT_NODE, Nokogiri::XML::Node::CDATA_SECTION_NODE,Nokogiri::XML::Node::COMMENT_NODE | |
| 29 | - return Scrubber::CONTINUE | |
| 30 | - end | |
| 31 | - Scrubber::STOP | |
| 32 | - end | |
| 33 | - | |
| 34 | - end | |
| 35 | -end | ... | ... |
test/unit/comment_test.rb
| ... | ... | @@ -188,7 +188,8 @@ class CommentTest < ActiveSupport::TestCase |
| 188 | 188 | owner = create_user('testuser').person |
| 189 | 189 | article = owner.articles.create!(:name => 'test', :body => '...') |
| 190 | 190 | javascript = "<script>alert('XSS')</script>" |
| 191 | - comment = create(Comment, :article => article, :name => javascript, :title => javascript, :body => javascript, :email => 'cracker@test.org') | |
| 191 | + comment = Comment.new(:source => article, :name => javascript, :title => javascript, :body => javascript, :email => 'cracker@test.org') | |
| 192 | + comment.valid? | |
| 192 | 193 | assert_no_match(/<script>/, comment.name) |
| 193 | 194 | end |
| 194 | 195 | ... | ... |
vendor/plugins/xss_terminate/lib/xss_terminate.rb
| ... | ... | @@ -44,15 +44,15 @@ module XssTerminate |
| 44 | 44 | puts field |
| 45 | 45 | self[field].each_key { |key| |
| 46 | 46 | key = key.to_sym |
| 47 | - self[field][key] = sanitizer.sanitize(self[field][key]) | |
| 47 | + self[field][key] = sanitizer.sanitize(self[field][key], scrubber: Rails::Html::PermitScrubber.new, encode_special_chars: false) | |
| 48 | 48 | } |
| 49 | 49 | else |
| 50 | 50 | if self[field] |
| 51 | - self[field] = sanitizer.sanitize(self[field]) | |
| 51 | + self[field] = sanitizer.sanitize(self[field], scrubber: Rails::Html::PermitScrubber.new, encode_special_chars: false) | |
| 52 | 52 | else |
| 53 | 53 | value = self.send("#{field}") |
| 54 | 54 | return unless value |
| 55 | - value = sanitizer.sanitize(value) | |
| 55 | + value = sanitizer.sanitize(value, scrubber: Rails::Html::PermitScrubber.new, encode_special_chars: false) | |
| 56 | 56 | self.send("#{field}=", value) |
| 57 | 57 | end |
| 58 | 58 | end |
| ... | ... | @@ -69,7 +69,7 @@ module XssTerminate |
| 69 | 69 | end |
| 70 | 70 | |
| 71 | 71 | def sanitize_fields_with_full |
| 72 | - sanitizer = ActionView::Base.full_sanitizer | |
| 72 | + sanitizer = Rails::Html::FullSanitizer.new | |
| 73 | 73 | columns, columns_serialized = sanitize_columns(:full) |
| 74 | 74 | columns.each do |column| |
| 75 | 75 | sanitize_field(sanitizer, column.to_sym, columns_serialized.include?(column)) |
| ... | ... | @@ -77,7 +77,7 @@ module XssTerminate |
| 77 | 77 | end |
| 78 | 78 | |
| 79 | 79 | def sanitize_fields_with_white_list |
| 80 | - sanitizer = ActionView::Base.white_list_sanitizer | |
| 80 | + sanitizer = Rails::Html::WhiteListSanitizer.new | |
| 81 | 81 | columns, columns_serialized = sanitize_columns(:white_list) |
| 82 | 82 | columns.each do |column| |
| 83 | 83 | sanitize_field(sanitizer, column.to_sym, columns_serialized.include?(column)) | ... | ... |