Download as
Browse Code »

Commit db65013977f059e29757ed115180fa43d43a5d67

Authored by Braulio Bhavamitra
1 parent 31b28c41
Exists in master and in 20 other branches add_member_task_reject_details, admin_visible_profile, community_notifications, fix_comments_pagination, fix_rails4_organization_ratings, fix_sign_up_form, follow_step_fix, forum_topic_creation, mirror_block_improvements, multi_env_on_remote_user, new_security, organization_ratings_improvements, profile_api_improvements, ratings_minor_fixes, send_email_to_admins, stable-spb-1.4, stable-spb-1.5, suggest_rejected_value, web_steps_improvements, xss_terminate_custom_options

rails4: use new rails sanitizer (loofah)

Showing 6 changed files with 55 additions and 18 deletions   Show diff stats
app/models/tiny_mce_article.rb
  Diff comments   View file @db65013
@@ -9,7 +9,7 @@ class TinyMceArticle < TextArticle @@ -9,7 +9,7 @@ class TinyMceArticle < TextArticle
9 def self.description 9 def self.description
10 _('Not accessible for visually impaired users.') 10 _('Not accessible for visually impaired users.')
11 end 11 end
12 - 12 +
13 xss_terminate :only => [ ] 13 xss_terminate :only => [ ]
14 14
15 xss_terminate :only => [ :name, :abstract, :body ], :with => 'white_list', :on => 'validation' 15 xss_terminate :only => [ :name, :abstract, :body ], :with => 'white_list', :on => 'validation'
config/application.rb
  Diff comments   View file @db65013
@@ -15,12 +15,6 @@ module Noosfero @@ -15,12 +15,6 @@ module Noosfero
15 15
16 require 'noosfero/plugin' 16 require 'noosfero/plugin'
17 17
18 - # Adds custom attributes to the Set of allowed html attributes for the #sanitize helper  
19 - config.action_view.sanitized_allowed_attributes = 'align', 'border', 'alt', 'vspace', 'hspace', 'width', 'heigth', 'value', 'type', 'data', 'style', 'target', 'codebase', 'archive', 'classid', 'code', 'flashvars', 'scrolling', 'frameborder', 'controls', 'autoplay', 'colspan', 'rowspan'  
20 -  
21 - # Adds custom tags to the Set of allowed html tags for the #sanitize helper  
22 - config.action_view.sanitized_allowed_tags = 'object', 'embed', 'param', 'table', 'tr', 'th', 'td', 'applet', 'comment', 'iframe', 'audio', 'video', 'source'  
23 -  
24 config.action_controller.include_all_helpers = false 18 config.action_controller.include_all_helpers = false
25 19
26 # Settings in config/environments/* take precedence over those specified here. 20 # Settings in config/environments/* take precedence over those specified here.
config/initializers/sanitizer.rb 0 → 100644
  Diff comments   View file @db65013
@@ -0,0 +1,35 @@ @@ -0,0 +1,35 @@
  1 +require 'loofah/helpers'
  2 +
  3 +ActionView::Base.full_sanitizer = Loofah::Helpers::ActionView::FullSanitizer.new
  4 +ActionView::Base.white_list_sanitizer = Loofah::Helpers::ActionView::WhiteListSanitizer.new
  5 +
  6 +Loofah::HTML5::WhiteList::ALLOWED_ELEMENTS_WITH_LIBXML2.merge %w[
  7 + img object embed param table tr th td applet comment iframe audio video source
  8 +]
  9 +
  10 +Loofah::HTML5::WhiteList::ALLOWED_ATTRIBUTES.merge %w[
  11 + align border alt vspace hspace width heigth value type data
  12 + style target codebase archive classid code flashvars scrolling frameborder controls autoplay colspan
  13 +]
  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/test_helper.rb
  Diff comments   View file @db65013
@@ -135,16 +135,23 @@ class ActiveSupport::TestCase @@ -135,16 +135,23 @@ class ActiveSupport::TestCase
135 assert !text.index('<'), "Text '#{text}' expected to be sanitized" 135 assert !text.index('<'), "Text '#{text}' expected to be sanitized"
136 end 136 end
137 137
  138 + def find_tag_in_string text, options
  139 + doc = Nokogiri::HTML.fragment text
  140 + tag = doc.css(options[:tag]).first
  141 + attributes = {}; tag.attributes.each do |a, v|
  142 + a = a.to_sym
  143 + next unless options[:attributes].has_key? a
  144 + attributes[a] = v.value
  145 + end
  146 + tag if (tag and attributes == options[:attributes])
  147 + end
  148 +
138 def assert_tag_in_string(text, options) 149 def assert_tag_in_string(text, options)
139 - doc = HTML::Document.new(text, false, false)  
140 - tag = doc.find(options)  
141 - assert tag, "expected tag #{options.inspect}, but not found in #{text.inspect}" 150 + assert find_tag_in_string(text, options), "expected tag #{options.inspect}, but not found in #{text.inspect}"
142 end 151 end
143 152
144 def assert_no_tag_in_string(text, options) 153 def assert_no_tag_in_string(text, options)
145 - doc = HTML::Document.new(text, false, false)  
146 - tag = doc.find(options)  
147 - assert !tag, "expected no tag #{options.inspect}, but tag found in #{text.inspect}" 154 + assert !find_tag_in_string(text, options), "expected no tag #{options.inspect}, but tag found in #{text.inspect}"
148 end 155 end
149 156
150 def assert_order(reference, original) 157 def assert_order(reference, original)
test/unit/article_test.rb
  Diff comments   View file @db65013
@@ -935,7 +935,7 @@ class ArticleTest &lt; ActiveSupport::TestCase @@ -935,7 +935,7 @@ class ArticleTest &lt; ActiveSupport::TestCase
935 article.name = "<h1 Malformed >> html >< tag" 935 article.name = "<h1 Malformed >> html >< tag"
936 article.valid? 936 article.valid?
937 937
938 - assert_no_match /[<>]/, article.name 938 + assert_equal '<h1>&gt; html &gt;</h1>', article.name
939 end 939 end
940 940
941 should 'return truncated title in short_title' do 941 should 'return truncated title in short_title' do
@@ -1734,6 +1734,7 @@ class ArticleTest &lt; ActiveSupport::TestCase @@ -1734,6 +1734,7 @@ class ArticleTest &lt; ActiveSupport::TestCase
1734 1734
1735 should 'store first image in tracked action' do 1735 should 'store first image in tracked action' do
1736 a = create TinyMceArticle, :name => 'Tracked Article', :body => '<p>Foo<img src="foo.png" />Bar</p>', :profile_id => profile.id 1736 a = create TinyMceArticle, :name => 'Tracked Article', :body => '<p>Foo<img src="foo.png" />Bar</p>', :profile_id => profile.id
  1737 + assert_equal 'foo.png', a.first_image
1737 assert_equal 'foo.png', ActionTracker::Record.last.get_first_image 1738 assert_equal 'foo.png', ActionTracker::Record.last.get_first_image
1738 end 1739 end
1739 1740
test/unit/tiny_mce_article_test.rb
  Diff comments   View file @db65013
@@ -82,16 +82,16 @@ class TinyMceArticleTest &lt; ActiveSupport::TestCase @@ -82,16 +82,16 @@ class TinyMceArticleTest &lt; ActiveSupport::TestCase
82 assert_no_tag_in_string article.body, :tag => 'iframe', :attributes => { :src => "http://untrusted_site.com/videos.ogg"} 82 assert_no_tag_in_string article.body, :tag => 'iframe', :attributes => { :src => "http://untrusted_site.com/videos.ogg"}
83 end 83 end
84 84
85 - should 'remove iframe if it has 2 or more src' do 85 + should 'consider first src if there is 2 or more src' do
86 assert_includes Environment.default.trusted_sites_for_iframe, 'itheora.org' 86 assert_includes Environment.default.trusted_sites_for_iframe, 'itheora.org'
87 87
88 article = create(TinyMceArticle, :profile => profile, :name => 'article', :abstract => 'abstract', :body => "<iframe src='http://itheora.org/videos.ogg' src='http://untrusted_site.com/videos.ogg'></iframe>") 88 article = create(TinyMceArticle, :profile => profile, :name => 'article', :abstract => 'abstract', :body => "<iframe src='http://itheora.org/videos.ogg' src='http://untrusted_site.com/videos.ogg'></iframe>")
89 - assert_equal '', article.body 89 + assert_tag_in_string article.body, :tag => 'iframe', :attributes => { :src => "http://itheora.org/videos.ogg"}
90 end 90 end
91 91
92 should 'not sanitize html comments' do 92 should 'not sanitize html comments' do
93 article = TinyMceArticle.new 93 article = TinyMceArticle.new
94 - article.body = '<p><!-- <asdf> << aasdfa >>> --> <h1> Wellformed html code </h1>' 94 + article.body = '<!-- <asdf> << aasdfa >>> --> <h1> Wellformed html code </h1>'
95 article.valid? 95 article.valid?
96 96
97 assert_match /<!-- .* --> <h1> Wellformed html code <\/h1>/, article.body 97 assert_match /<!-- .* --> <h1> Wellformed html code <\/h1>/, article.body
@@ -232,7 +232,7 @@ end @@ -232,7 +232,7 @@ end
232 :profile => profile 232 :profile => profile
233 ) 233 )
234 assert_tag_in_string article.body, :tag => 'table', 234 assert_tag_in_string article.body, :tag => 'table',
235 - :attributes => { :colspan => 2, :rowspan => 3 } 235 + :attributes => { :colspan => '2', :rowspan => '3' }
236 end 236 end
237 237
238 end 238 end