From 3fe538f12e2bbe1eb4e77dc91abd93bfc9963da5 Mon Sep 17 00:00:00 2001 From: Daniela Soares Feitosa Date: Sat, 14 Aug 2010 23:26:24 -0300 Subject: [PATCH] Allowed environment to set trusted sites on iframes --- app/models/environment.rb | 5 +++++ app/models/event.rb | 3 +++ app/models/folder.rb | 3 +++ app/models/product.rb | 3 +++ app/models/profile.rb | 4 ++++ app/models/tiny_mce_article.rb | 4 ++++ lib/white_list_filter.rb | 37 +++++++++++++++++++++++++++++++++++++ public/stylesheets/application.css | 1 - test/unit/environment_test.rb | 11 +++++++++++ test/unit/event_test.rb | 1 - test/unit/tiny_mce_article_test.rb | 53 +++++++++++++++++++++++++++++++++++++++-------------- test/unit/white_list_filter_test.rb | 54 ++++++++++++++++++++++++++++++++++++++++++++++++++++++ vendor/plugins/white_list_sanitizer_unescape_before_reescape/init.rb | 7 ------- 13 files changed, 163 insertions(+), 23 deletions(-) create mode 100644 lib/white_list_filter.rb create mode 100644 test/unit/white_list_filter_test.rb diff --git a/app/models/environment.rb b/app/models/environment.rb index 3575f3f..cc4a37b 100644 --- a/app/models/environment.rb +++ b/app/models/environment.rb @@ -219,6 +219,8 @@ class Environment < ActiveRecord::Base settings_items :currency_separator, :type => String, :default => '.' settings_items :currency_delimiter, :type => String, :default => ',' + settings_items :trusted_sites_for_iframe, :type => Array, :default => ['itheora.org', 'tv.softwarelivre.org', 'stream.softwarelivre.org'] + def news_amount_by_folder=(amount) settings[:news_amount_by_folder] = amount.to_i end @@ -468,6 +470,9 @@ class Environment < ActiveRecord::Base xss_terminate :only => [ :message_for_disabled_enterprise ], :with => 'white_list', :on => 'validation' + include WhiteListFilter + filter_iframes :message_for_disabled_enterprise, :whitelist => lambda { trusted_sites_for_iframe } + # ################################################# # Business logic in general diff --git a/app/models/event.rb b/app/models/event.rb index b9bd376..0ef4429 100644 --- a/app/models/event.rb +++ b/app/models/event.rb @@ -26,6 +26,9 @@ class Event < Article {:conditions => ['start_date = :date AND end_date IS NULL OR (start_date <= :date AND end_date >= :date)', {:date => date}]} } + include WhiteListFilter + filter_iframes :description, :link, :address, :whitelist => lambda { profile && profile.environment && profile.environment.trusted_sites_for_iframe } + def self.description _('A calendar event') end diff --git a/app/models/folder.rb b/app/models/folder.rb index 06da77f..4a1138e 100644 --- a/app/models/folder.rb +++ b/app/models/folder.rb @@ -6,6 +6,9 @@ class Folder < Article xss_terminate :only => [ :body ], :with => 'white_list', :on => 'validation' + include WhiteListFilter + filter_iframes :body, :whitelist => lambda { profile && profile.environment && profile.environment.trusted_sites_for_iframe } + def self.select_views [[_('Folder'), 'folder'], [_('Image gallery'), 'image_gallery']] end diff --git a/app/models/product.rb b/app/models/product.rb index 20436ca..ddce917 100644 --- a/app/models/product.rb +++ b/app/models/product.rb @@ -42,6 +42,9 @@ class Product < ActiveRecord::Base acts_as_mappable + include WhiteListFilter + filter_iframes :description, :whitelist => lambda { enterprise && enterprise.environment && enterprise.environment.trusted_sites_for_iframe } + def self.units { _('Litre') => 'litre', diff --git a/app/models/profile.rb b/app/models/profile.rb index 6cbfc7f..eb4fd96 100644 --- a/app/models/profile.rb +++ b/app/models/profile.rb @@ -314,6 +314,10 @@ class Profile < ActiveRecord::Base xss_terminate :only => [ :name, :nickname, :address, :contact_phone, :description ], :on => 'validation' xss_terminate :only => [ :custom_footer, :custom_header ], :with => 'white_list', :on => 'validation' + include WhiteListFilter + filter_iframes :custom_header, :custom_footer, :whitelist => lambda { environment && environment.trusted_sites_for_iframe } + + # returns the contact email for this profile. # # Subclasses may -- and should -- override this method. diff --git a/app/models/tiny_mce_article.rb b/app/models/tiny_mce_article.rb index 489661f..491852a 100644 --- a/app/models/tiny_mce_article.rb +++ b/app/models/tiny_mce_article.rb @@ -9,6 +9,10 @@ class TinyMceArticle < TextArticle end xss_terminate :except => [ :abstract, :body ] + xss_terminate :only => [ :abstract, :body ], :with => 'white_list', :on => 'validation' + include WhiteListFilter + filter_iframes :abstract, :body, :whitelist => lambda { profile && profile.environment && profile.environment.trusted_sites_for_iframe } + end diff --git a/lib/white_list_filter.rb b/lib/white_list_filter.rb new file mode 100644 index 0000000..33da203 --- /dev/null +++ b/lib/white_list_filter.rb @@ -0,0 +1,37 @@ +module WhiteListFilter + + def check_iframe_on_content(content, trusted_sites) + if content.blank? || !content.include?('iframe') + return content + end + content.gsub!(/]*>\s*<\/iframe>/i) do |iframe| + result = '' + unless iframe =~ /src=['"].*src=['"]/ + trusted_sites.each do |trusted_site| + re_dom = trusted_site.gsub('.', '\.') + if iframe =~ /src=["']https?:\/\/(www\.)?#{re_dom}\// + result = iframe + end + end + end + result + end + content + end + + module ClassMethods + def filter_iframes(*opts) + options = opts.pop + white_list_method = options[:whitelist] + opts.each do |field| + before_validation do |obj| + obj.check_iframe_on_content(obj.send(field), obj.instance_eval(&white_list_method)) + end + end + end + end + + def self.included(c) + c.send(:extend, WhiteListFilter::ClassMethods) + end +end diff --git a/public/stylesheets/application.css b/public/stylesheets/application.css index 2199302..cda8e9f 100644 --- a/public/stylesheets/application.css +++ b/public/stylesheets/application.css @@ -1444,7 +1444,6 @@ input.disabled { ***********************************************************/ .block iframe { - width: 100%; border: none; } diff --git a/test/unit/environment_test.rb b/test/unit/environment_test.rb index 2e8ce4c..c11b661 100644 --- a/test/unit/environment_test.rb +++ b/test/unit/environment_test.rb @@ -1074,4 +1074,15 @@ class EnvironmentTest < Test::Unit::TestCase assert_equal 15, env.profile_cache_in_minutes end + should 'have a list of trusted sites by default' do + assert_equal ['itheora.org', 'tv.softwarelivre.org', 'stream.softwarelivre.org'], Environment.new.trusted_sites_for_iframe + end + + should 'have a list of trusted sites' do + e = Environment.default + e.trusted_sites_for_iframe = ['trusted.site.org'] + e.save! + + assert_equal ['trusted.site.org'], Environment.default.trusted_sites_for_iframe + end end diff --git a/test/unit/event_test.rb b/test/unit/event_test.rb index 72cffb6..c5f519c 100644 --- a/test/unit/event_test.rb +++ b/test/unit/event_test.rb @@ -265,5 +265,4 @@ class EventTest < ActiveSupport::TestCase assert_match /

Wellformed html code <\/h1>/, event.description assert_match /

Wellformed html code <\/h1>/, event.address end - end diff --git a/test/unit/tiny_mce_article_test.rb b/test/unit/tiny_mce_article_test.rb index c2b21b8..cf14259 100644 --- a/test/unit/tiny_mce_article_test.rb +++ b/test/unit/tiny_mce_article_test.rb @@ -54,19 +54,49 @@ class TinyMceArticleTest < Test::Unit::TestCase assert_equal "the just for ie... ", article.body end - should 'not mess with ") - assert_equal "", article.body + should 'remove iframe if it is not from a trusted site' do + article = TinyMceArticle.create!(:profile => profile, :name => 'article', :abstract => 'abstract', :body => "") + assert_equal "", article.body end - should 'remove iframe if it is not from itheora or softwarelivre' do - article = TinyMceArticle.create!(:profile => profile, :name => 'article', :abstract => 'abstract', :body => "") - assert_equal "", article.body + should 'not mess with ") + assert_tag_in_string article.body, :tag => 'iframe', :attributes => { :src => "http://itheora.org/demo/index.php?v=example.ogv"} end - should 'allow iframe if it is from stream.softwarelivre.org' do - article = TinyMceArticle.create!(:profile => profile, :name => 'article', :abstract => 'abstract', :body => "") - assert_equal "", article.body + should 'allow iframe if it is from stream.softwarelivre.org by default' do + assert_includes Environment.default.trusted_sites_for_iframe, 'stream.softwarelivre.org' + article = TinyMceArticle.create!(:profile => profile, :name => 'article', :abstract => 'abstract', :body => "") + assert_tag_in_string article.body, :tag => 'iframe', :attributes => { :src => "http://stream.softwarelivre.org/fisl10/sites/default/files/videos.ogg"} + end + + should 'allow iframe if it is from tv.softwarelivre.org by default' do + assert_includes Environment.default.trusted_sites_for_iframe, 'tv.softwarelivre.org' + article = TinyMceArticle.create!(:profile => profile, :name => 'article', :abstract => 'abstract', :body => "") + assert_tag_in_string article.body, :tag => 'iframe', :attributes => { :src => "http://tv.softwarelivre.org/embed/1170", :width => "482", :height => "406", :align => "right", :frameborder => "0", :scrolling => "no"} + end + + should 'allow iframe if it is from a trusted site' do + env = Environment.default + env.trusted_sites_for_iframe = ['avideosite.com'] + env.save + assert_includes Environment.default.trusted_sites_for_iframe, 'avideosite.com' + article = TinyMceArticle.create!(:profile => profile, :name => 'article', :abstract => 'abstract', :body => "") + assert_tag_in_string article.body, :tag => 'iframe', :attributes => { :src => "http://avideosite.com/videos.ogg"} + end + + should 'remove only the iframe from untrusted site' do + article = TinyMceArticle.create!(:profile => profile, :name => 'article', :abstract => 'abstract', :body => "") + assert_tag_in_string article.body, :tag => 'iframe', :attributes => { :src => "http://stream.softwarelivre.org/videos.ogg"} + assert_no_tag_in_string article.body, :tag => 'iframe', :attributes => { :src => "http://untrusted_site.com/videos.ogg"} + end + + should 'remove iframe if it has 2 or more src' do + assert_includes Environment.default.trusted_sites_for_iframe, 'itheora.org' + + article = TinyMceArticle.create!(:profile => profile, :name => 'article', :abstract => 'abstract', :body => "") + assert_equal '', article.body end #TinymMCE convert config={"key":(.*)} in config={"key":(.*)} @@ -83,9 +113,4 @@ class TinyMceArticleTest < Test::Unit::TestCase assert_match /

Wellformed html code <\/h1>/, article.body end - should 'allow iframe if it is from tv.softwarelivre.org' do - article = TinyMceArticle.create!(:profile => profile, :name => 'article', :abstract => 'abstract', :body => "") - assert_tag_in_string article.body, :tag => 'iframe', :attributes => { :src => "http://tv.softwarelivre.org/embed/1170", :width => "482", :height => "406", :align => "right", :frameborder => "0", :scrolling => "no"} - end - end diff --git a/test/unit/white_list_filter_test.rb b/test/unit/white_list_filter_test.rb new file mode 100644 index 0000000..c1cd44c --- /dev/null +++ b/test/unit/white_list_filter_test.rb @@ -0,0 +1,54 @@ +require File.dirname(__FILE__) + '/../test_helper' + +class WhiteListFilterTest < Test::Unit::TestCase + + include WhiteListFilter + + def environment + @environment ||= Environment.default + end + + should 'remove iframe if it is not from a trusted site' do + content = "" + assert_equal "", check_iframe_on_content(content, environment.trusted_sites_for_iframe) + end + + should 'not mess with ", check_iframe_on_content(content, environment.trusted_sites_for_iframe) + end + + should 'allow iframe if it is from stream.softwarelivre.org by default' do + assert_includes Environment.default.trusted_sites_for_iframe, 'stream.softwarelivre.org' + content = "" + assert_equal "", check_iframe_on_content(content, environment.trusted_sites_for_iframe) + end + + should 'allow iframe if it is from tv.softwarelivre.org by default' do + assert_includes Environment.default.trusted_sites_for_iframe, 'tv.softwarelivre.org' + content = "" + assert_equal "", check_iframe_on_content(content, environment.trusted_sites_for_iframe) + end + + should 'allow iframe if it is from a trusted site' do + env = Environment.default + env.trusted_sites_for_iframe = ['avideosite.com'] + env.save + assert_includes Environment.default.trusted_sites_for_iframe, 'avideosite.com' + content = "" + assert_equal "", check_iframe_on_content(content, environment.trusted_sites_for_iframe) + end + + should 'remove only the iframe from untrusted site' do + content = "" + assert_equal "", check_iframe_on_content(content, environment.trusted_sites_for_iframe) + end + + should 'remove iframe if it has 2 or more src' do + assert_includes Environment.default.trusted_sites_for_iframe, 'itheora.org' + + content = "" + assert_equal '', check_iframe_on_content(content, environment.trusted_sites_for_iframe) + end +end diff --git a/vendor/plugins/white_list_sanitizer_unescape_before_reescape/init.rb b/vendor/plugins/white_list_sanitizer_unescape_before_reescape/init.rb index 5c5b432..a5105ca 100644 --- a/vendor/plugins/white_list_sanitizer_unescape_before_reescape/init.rb +++ b/vendor/plugins/white_list_sanitizer_unescape_before_reescape/init.rb @@ -11,13 +11,6 @@ HTML::WhiteListSanitizer.module_eval do final_text = text.gsub(/<!/, '(.*)/, '\1') #FIX for itheora comments - if final_text =~ /iframe/ - itheora_video = // - sl_video = // - unless (final_text =~ itheora_video || final_text =~ sl_video) - final_text = final_text.gsub(//, '') - end - end final_text = final_text.gsub(/&quot;/, '"') #FIX problems with archive.org final_text end -- libgit2 0.21.2