Commit 34593e717c723526ec7eabd8b49f1fd016f309ec
Exists in
master
and in
28 other branches
Merge branch 'stable'
Showing
31 changed files
with
316 additions
and
35 deletions
Show diff stats
app/helpers/folder_helper.rb
| @@ -19,7 +19,7 @@ module FolderHelper | @@ -19,7 +19,7 @@ module FolderHelper | ||
| 19 | def display_article_in_listing(article, recursive = false, level = 0) | 19 | def display_article_in_listing(article, recursive = false, level = 0) |
| 20 | result = content_tag( | 20 | result = content_tag( |
| 21 | 'tr', | 21 | 'tr', |
| 22 | - content_tag('td', link_to((' ' * (level * 4) ) + image_tag(icon_for_article(article)) + article.name, article.url.merge(:view => true)))+ | 22 | + content_tag('td', link_to((' ' * (level * 4) ) + image_tag(icon_for_article(article)) + short_filename(article.name), article.url.merge(:view => true)))+ |
| 23 | content_tag('td', show_date(article.updated_at), :class => 'last-update'), | 23 | content_tag('td', show_date(article.updated_at), :class => 'last-update'), |
| 24 | :class => 'sitemap-item' | 24 | :class => 'sitemap-item' |
| 25 | ) | 25 | ) |
| @@ -69,4 +69,11 @@ module FolderHelper | @@ -69,4 +69,11 @@ module FolderHelper | ||
| 69 | _('Edit folder') | 69 | _('Edit folder') |
| 70 | end | 70 | end |
| 71 | 71 | ||
| 72 | + def short_filename(filename, limit_chars = 43) | ||
| 73 | + return filename if filename.size <= limit_chars | ||
| 74 | + extname = File.extname(filename) | ||
| 75 | + basename = File.basename(filename,extname) | ||
| 76 | + str_complement = '(...)' | ||
| 77 | + return basename[0..(limit_chars - extname.size - str_complement.size - 1)] + str_complement + extname | ||
| 78 | + end | ||
| 72 | end | 79 | end |
app/models/article.rb
| @@ -26,7 +26,7 @@ class Article < ActiveRecord::Base | @@ -26,7 +26,7 @@ class Article < ActiveRecord::Base | ||
| 26 | article.published_at = article.created_at if article.published_at.nil? | 26 | article.published_at = article.created_at if article.published_at.nil? |
| 27 | end | 27 | end |
| 28 | 28 | ||
| 29 | - xss_terminate :only => [ :name ] | 29 | + xss_terminate :only => [ :name ], :on => 'validation' |
| 30 | 30 | ||
| 31 | named_scope :in_category, lambda { |category| | 31 | named_scope :in_category, lambda { |category| |
| 32 | {:include => 'categories', :conditions => { 'categories.id' => category.id }} | 32 | {:include => 'categories', :conditions => { 'categories.id' => category.id }} |
app/models/comment.rb
| @@ -17,7 +17,7 @@ class Comment < ActiveRecord::Base | @@ -17,7 +17,7 @@ class Comment < ActiveRecord::Base | ||
| 17 | end | 17 | end |
| 18 | end | 18 | end |
| 19 | 19 | ||
| 20 | - xss_terminate :only => [ :body, :title, :name ] | 20 | + xss_terminate :only => [ :body, :title, :name ], :on => 'validation' |
| 21 | 21 | ||
| 22 | def author_name | 22 | def author_name |
| 23 | if author | 23 | if author |
app/models/community.rb
| @@ -5,8 +5,6 @@ class Community < Organization | @@ -5,8 +5,6 @@ class Community < Organization | ||
| 5 | settings_items :language | 5 | settings_items :language |
| 6 | settings_items :zip_code, :city, :state, :country | 6 | settings_items :zip_code, :city, :state, :country |
| 7 | 7 | ||
| 8 | - xss_terminate :only => [ :name, :address, :contact_phone, :description ] | ||
| 9 | - | ||
| 10 | before_create do |community| | 8 | before_create do |community| |
| 11 | community.moderated_articles = true if community.environment.enabled?('organizations_are_moderated_by_default') | 9 | community.moderated_articles = true if community.environment.enabled?('organizations_are_moderated_by_default') |
| 12 | end | 10 | end |
| @@ -22,6 +20,8 @@ class Community < Organization | @@ -22,6 +20,8 @@ class Community < Organization | ||
| 22 | community | 20 | community |
| 23 | end | 21 | end |
| 24 | 22 | ||
| 23 | + xss_terminate :only => [ :name, :address, :contact_phone, :description ], :on => 'validation' | ||
| 24 | + | ||
| 25 | FIELDS = %w[ | 25 | FIELDS = %w[ |
| 26 | city | 26 | city |
| 27 | state | 27 | state |
app/models/consumption.rb
| @@ -4,6 +4,6 @@ class Consumption < ActiveRecord::Base | @@ -4,6 +4,6 @@ class Consumption < ActiveRecord::Base | ||
| 4 | 4 | ||
| 5 | validates_uniqueness_of :product_category_id, :scope => :profile_id | 5 | validates_uniqueness_of :product_category_id, :scope => :profile_id |
| 6 | 6 | ||
| 7 | - xss_terminate :only => [ :aditional_specifications ] | 7 | + xss_terminate :only => [ :aditional_specifications ], :on => 'validation' |
| 8 | 8 | ||
| 9 | end | 9 | end |
app/models/environment.rb
| @@ -471,7 +471,7 @@ class Environment < ActiveRecord::Base | @@ -471,7 +471,7 @@ class Environment < ActiveRecord::Base | ||
| 471 | 471 | ||
| 472 | validates_format_of :contact_email, :with => Noosfero::Constants::EMAIL_FORMAT, :if => (lambda { |record| ! record.contact_email.blank? }) | 472 | validates_format_of :contact_email, :with => Noosfero::Constants::EMAIL_FORMAT, :if => (lambda { |record| ! record.contact_email.blank? }) |
| 473 | 473 | ||
| 474 | - xss_terminate :only => [ :message_for_disabled_enterprise ], :with => 'white_list' | 474 | + xss_terminate :only => [ :message_for_disabled_enterprise ], :with => 'white_list', :on => 'validation' |
| 475 | 475 | ||
| 476 | 476 | ||
| 477 | # ################################################# | 477 | # ################################################# |
app/models/event.rb
| @@ -6,7 +6,8 @@ class Event < Article | @@ -6,7 +6,8 @@ class Event < Article | ||
| 6 | settings_items :link, :type => :string | 6 | settings_items :link, :type => :string |
| 7 | settings_items :address, :type => :string | 7 | settings_items :address, :type => :string |
| 8 | 8 | ||
| 9 | - xss_terminate :only => [ :description, :link, :address ], :with => 'white_list' | 9 | + xss_terminate :only => [ :link ], :on => 'validation' |
| 10 | + xss_terminate :only => [ :description, :link, :address ], :with => 'white_list', :on => 'validation' | ||
| 10 | 11 | ||
| 11 | validates_presence_of :title, :start_date | 12 | validates_presence_of :title, :start_date |
| 12 | 13 |
app/models/folder.rb
| @@ -4,7 +4,7 @@ class Folder < Article | @@ -4,7 +4,7 @@ class Folder < Article | ||
| 4 | 4 | ||
| 5 | settings_items :view_as, :type => :string, :default => 'folder' | 5 | settings_items :view_as, :type => :string, :default => 'folder' |
| 6 | 6 | ||
| 7 | - xss_terminate :only => [ :body ], :with => 'white_list' | 7 | + xss_terminate :only => [ :body ], :with => 'white_list', :on => 'validation' |
| 8 | 8 | ||
| 9 | def self.select_views | 9 | def self.select_views |
| 10 | [[_('Folder'), 'folder'], [_('Image gallery'), 'image_gallery']] | 10 | [[_('Folder'), 'folder'], [_('Image gallery'), 'image_gallery']] |
app/models/organization.rb
| @@ -77,7 +77,7 @@ class Organization < Profile | @@ -77,7 +77,7 @@ class Organization < Profile | ||
| 77 | 77 | ||
| 78 | validates_format_of :contact_email, :with => Noosfero::Constants::EMAIL_FORMAT, :if => (lambda { |org| !org.contact_email.blank? }) | 78 | validates_format_of :contact_email, :with => Noosfero::Constants::EMAIL_FORMAT, :if => (lambda { |org| !org.contact_email.blank? }) |
| 79 | 79 | ||
| 80 | - xss_terminate :only => [ :acronym, :contact_person, :contact_email, :legal_form, :economic_activity, :management_information ] | 80 | + xss_terminate :only => [ :acronym, :contact_person, :contact_email, :legal_form, :economic_activity, :management_information ], :on => 'validation' |
| 81 | 81 | ||
| 82 | # Yes, organizations have members. | 82 | # Yes, organizations have members. |
| 83 | # | 83 | # |
app/models/product.rb
| @@ -31,7 +31,7 @@ class Product < ActiveRecord::Base | @@ -31,7 +31,7 @@ class Product < ActiveRecord::Base | ||
| 31 | 31 | ||
| 32 | acts_as_searchable :fields => [ :name, :description, :category_full_name ] | 32 | acts_as_searchable :fields => [ :name, :description, :category_full_name ] |
| 33 | 33 | ||
| 34 | - xss_terminate :only => [ :name, :description ] | 34 | + xss_terminate :only => [ :name, :description ], :on => 'validation' |
| 35 | 35 | ||
| 36 | acts_as_mappable | 36 | acts_as_mappable |
| 37 | 37 |
app/models/profile.rb
| @@ -294,8 +294,8 @@ class Profile < ActiveRecord::Base | @@ -294,8 +294,8 @@ class Profile < ActiveRecord::Base | ||
| 294 | self.save_without_validation! | 294 | self.save_without_validation! |
| 295 | end | 295 | end |
| 296 | 296 | ||
| 297 | - xss_terminate :only => [ :name, :nickname, :address, :contact_phone, :description ] | ||
| 298 | - xss_terminate :only => [ :custom_footer, :custom_header ], :with => 'white_list' | 297 | + xss_terminate :only => [ :name, :nickname, :address, :contact_phone, :description ], :on => 'validation' |
| 298 | + xss_terminate :only => [ :custom_footer, :custom_header ], :with => 'white_list', :on => 'validation' | ||
| 299 | 299 | ||
| 300 | # returns the contact email for this profile. | 300 | # returns the contact email for this profile. |
| 301 | # | 301 | # |
app/models/text_article.rb
app/models/tiny_mce_article.rb
| @@ -9,6 +9,6 @@ class TinyMceArticle < TextArticle | @@ -9,6 +9,6 @@ class TinyMceArticle < TextArticle | ||
| 9 | end | 9 | end |
| 10 | 10 | ||
| 11 | xss_terminate :except => [ :abstract, :body ] | 11 | xss_terminate :except => [ :abstract, :body ] |
| 12 | - xss_terminate :only => [ :abstract, :body ], :with => 'white_list' | 12 | + xss_terminate :only => [ :abstract, :body ], :with => 'white_list', :on => 'validation' |
| 13 | 13 | ||
| 14 | end | 14 | end |
app/models/validation_info.rb
| @@ -3,5 +3,5 @@ class ValidationInfo < ActiveRecord::Base | @@ -3,5 +3,5 @@ class ValidationInfo < ActiveRecord::Base | ||
| 3 | 3 | ||
| 4 | belongs_to :organization | 4 | belongs_to :organization |
| 5 | 5 | ||
| 6 | - xss_terminate :only => [ :validation_methodology, :restrictions ] | 6 | + xss_terminate :only => [ :validation_methodology, :restrictions ], :on => 'validation' |
| 7 | end | 7 | end |
app/views/cms/view.rhtml
| @@ -72,7 +72,7 @@ | @@ -72,7 +72,7 @@ | ||
| 72 | <tr> | 72 | <tr> |
| 73 | <td> | 73 | <td> |
| 74 | <%= image_tag(icon_for_article(item)) %> | 74 | <%= image_tag(icon_for_article(item)) %> |
| 75 | - <%= link_to item.name, :action => 'edit', :id => item.id %> | 75 | + <%= link_to short_filename(item.name,30), :action => 'edit', :id => item.id %> |
| 76 | </td> | 76 | </td> |
| 77 | <td> | 77 | <td> |
| 78 | <%= item.class.short_description %> | 78 | <%= item.class.short_description %> |
| @@ -0,0 +1,21 @@ | @@ -0,0 +1,21 @@ | ||
| 1 | +Feature: sitemap | ||
| 2 | + As a noosfero user | ||
| 3 | + I want to list articles | ||
| 4 | + | ||
| 5 | + Background: | ||
| 6 | + Given I am on the homepage | ||
| 7 | + And the following users | ||
| 8 | + | login | name | | ||
| 9 | + | joaosilva | Joao Silva | | ||
| 10 | + And the following files | ||
| 11 | + | owner | file | mime | | ||
| 12 | + | joaosilva | AGENDA_CULTURA_-_FESTA_DE_VAQUEIROS_PONTO_DE_SERRA_PRETA_BAIXA.txt | text/plain | | ||
| 13 | + | ||
| 14 | + Scenario: view a folder page | ||
| 15 | + When I am on /profile/joaosilva/sitemap | ||
| 16 | + Then I should see "AGENDA_CULTURA_-_FESTA_DE_VAQUEIRO(...).txt" | ||
| 17 | + | ||
| 18 | + Scenario: view the CMS | ||
| 19 | + Given I am logged in as "joaosilva" | ||
| 20 | + When I am on /myprofile/joaosilva/cms | ||
| 21 | + Then I should see "AGENDA_CULTURA_-_FEST(...).txt" |
test/fixtures/files/AGENDA_CULTURA_-_FESTA_DE_VAQUEIROS_PONTO_DE_SERRA_PRETA_BAIXA.txt
0 → 100644
| @@ -0,0 +1 @@ | @@ -0,0 +1 @@ | ||
| 1 | +test |
test/unit/application_helper_test.rb
| @@ -558,6 +558,18 @@ class ApplicationHelperTest < Test::Unit::TestCase | @@ -558,6 +558,18 @@ class ApplicationHelperTest < Test::Unit::TestCase | ||
| 558 | assert_equal environment.theme, current_theme | 558 | assert_equal environment.theme, current_theme |
| 559 | end | 559 | end |
| 560 | 560 | ||
| 561 | + should 'trunc to 15 chars the big filename' do | ||
| 562 | + assert_equal 'AGENDA(...).mp3', short_filename('AGENDA_CULTURA_-_FESTA_DE_VAQUEIROS_PONTO_DE_SERRA_PRETA_BAIXA.mp3',15) | ||
| 563 | + end | ||
| 564 | + | ||
| 565 | + should 'trunc to default limit the big filename' do | ||
| 566 | + assert_equal 'AGENDA_CULTURA_-_FESTA_DE_VAQUEIRO(...).mp3', short_filename('AGENDA_CULTURA_-_FESTA_DE_VAQUEIROS_PONTO_DE_SERRA_PRETA_BAIXA.mp3') | ||
| 567 | + end | ||
| 568 | + | ||
| 569 | + should 'does not trunc short filename' do | ||
| 570 | + assert_equal 'filename.mp3', short_filename('filename.mp3') | ||
| 571 | + end | ||
| 572 | + | ||
| 561 | protected | 573 | protected |
| 562 | 574 | ||
| 563 | def url_for(args = {}) | 575 | def url_for(args = {}) |
test/unit/article_test.rb
| @@ -859,4 +859,20 @@ class ArticleTest < Test::Unit::TestCase | @@ -859,4 +859,20 @@ class ArticleTest < Test::Unit::TestCase | ||
| 859 | assert_no_match /</, article.tags.last.name | 859 | assert_no_match /</, article.tags.last.name |
| 860 | end | 860 | end |
| 861 | 861 | ||
| 862 | + should 'sanitize name before validation' do | ||
| 863 | + article = Article.new | ||
| 864 | + article.name = "<h1 Bla </h1>" | ||
| 865 | + article.valid? | ||
| 866 | + | ||
| 867 | + assert article.errors.invalid?(:name) | ||
| 868 | + end | ||
| 869 | + | ||
| 870 | + should 'escape malformed html tags' do | ||
| 871 | + article = Article.new | ||
| 872 | + article.name = "<h1 Malformed >> html >< tag" | ||
| 873 | + article.valid? | ||
| 874 | + | ||
| 875 | + assert_no_match /[<>]/, article.name | ||
| 876 | + end | ||
| 877 | + | ||
| 862 | end | 878 | end |
test/unit/comment_test.rb
| @@ -187,4 +187,26 @@ class CommentTest < Test::Unit::TestCase | @@ -187,4 +187,26 @@ class CommentTest < Test::Unit::TestCase | ||
| 187 | assert_no_match(/<script>/, comment.name) | 187 | assert_no_match(/<script>/, comment.name) |
| 188 | end | 188 | end |
| 189 | 189 | ||
| 190 | + should 'sanitize required fields before validation' do | ||
| 191 | + owner = create_user('testuser').person | ||
| 192 | + article = owner.articles.create(:name => 'test', :body => '...') | ||
| 193 | + comment = article.comments.new(:title => '<h1 title </h1>', :body => '<h1 body </h1>', :name => '<h1 name </h1>', :email => 'cracker@test.org') | ||
| 194 | + comment.valid? | ||
| 195 | + | ||
| 196 | + assert comment.errors.invalid?(:name) | ||
| 197 | + assert comment.errors.invalid?(:title) | ||
| 198 | + assert comment.errors.invalid?(:body) | ||
| 199 | + end | ||
| 200 | + | ||
| 201 | + should 'escape malformed html tags' do | ||
| 202 | + owner = create_user('testuser').person | ||
| 203 | + article = owner.articles.create(:name => 'test', :body => '...') | ||
| 204 | + comment = article.comments.new(:title => '<h1 title </h1>>> sd f <<', :body => '<h1>> sdf><asd>< body </h1>', :name => '<h1 name </h1>>><<dfsf<sd', :email => 'cracker@test.org') | ||
| 205 | + comment.valid? | ||
| 206 | + | ||
| 207 | + assert_no_match /[<>]/, comment.title | ||
| 208 | + assert_no_match /[<>]/, comment.body | ||
| 209 | + assert_no_match /[<>]/, comment.name | ||
| 210 | + end | ||
| 211 | + | ||
| 190 | end | 212 | end |
test/unit/community_test.rb
| @@ -114,7 +114,7 @@ class CommunityTest < Test::Unit::TestCase | @@ -114,7 +114,7 @@ class CommunityTest < Test::Unit::TestCase | ||
| 114 | should 'require fields if community needs' do | 114 | should 'require fields if community needs' do |
| 115 | e = Environment.default | 115 | e = Environment.default |
| 116 | e.expects(:required_community_fields).returns(['contact_phone']).at_least_once | 116 | e.expects(:required_community_fields).returns(['contact_phone']).at_least_once |
| 117 | - community = Community.new(:environment => e) | 117 | + community = Community.new(:name => 'My community', :environment => e) |
| 118 | assert ! community.valid? | 118 | assert ! community.valid? |
| 119 | assert community.errors.invalid?(:contact_phone) | 119 | assert community.errors.invalid?(:contact_phone) |
| 120 | 120 | ||
| @@ -200,4 +200,19 @@ class CommunityTest < Test::Unit::TestCase | @@ -200,4 +200,19 @@ class CommunityTest < Test::Unit::TestCase | ||
| 200 | community.add_member(person) | 200 | community.add_member(person) |
| 201 | end | 201 | end |
| 202 | end | 202 | end |
| 203 | + | ||
| 204 | + should 'escape malformed html tags' do | ||
| 205 | + community = Community.new | ||
| 206 | + community.name = "<h1 Malformed >> html >< tag" | ||
| 207 | + community.address = "<h1 Malformed >,<<<asfdf> html >< tag" | ||
| 208 | + community.contact_phone = "<h1 Malformed<<> >> html >><>< tag" | ||
| 209 | + community.description = "<h1 Malformed /h1>>><<> html ><>h1< tag" | ||
| 210 | + community.valid? | ||
| 211 | + | ||
| 212 | + assert_no_match /[<>]/, community.name | ||
| 213 | + assert_no_match /[<>]/, community.address | ||
| 214 | + assert_no_match /[<>]/, community.contact_phone | ||
| 215 | + assert_no_match /[<>]/, community.description | ||
| 216 | + end | ||
| 217 | + | ||
| 203 | end | 218 | end |
test/unit/consumption_test.rb
| @@ -3,8 +3,12 @@ require File.dirname(__FILE__) + '/../test_helper' | @@ -3,8 +3,12 @@ require File.dirname(__FILE__) + '/../test_helper' | ||
| 3 | class ConsumptionTest < Test::Unit::TestCase | 3 | class ConsumptionTest < Test::Unit::TestCase |
| 4 | fixtures :consumptions | 4 | fixtures :consumptions |
| 5 | 5 | ||
| 6 | - # Replace this with your real tests. | ||
| 7 | - def test_truth | ||
| 8 | - assert true | 6 | + should 'escape malformed html tags' do |
| 7 | + consumption = Consumption.new | ||
| 8 | + consumption.aditional_specifications = "<h1 Malformed >> html >< tag" | ||
| 9 | + consumption.valid? | ||
| 10 | + | ||
| 11 | + assert_no_match /[<>]/, consumption.aditional_specifications | ||
| 9 | end | 12 | end |
| 13 | + | ||
| 10 | end | 14 | end |
test/unit/environment_test.rb
| @@ -878,4 +878,20 @@ class EnvironmentTest < Test::Unit::TestCase | @@ -878,4 +878,20 @@ class EnvironmentTest < Test::Unit::TestCase | ||
| 878 | assert_equal env.message_for_member_invitation, env.invitation_mail_template(community) | 878 | assert_equal env.message_for_member_invitation, env.invitation_mail_template(community) |
| 879 | end | 879 | end |
| 880 | 880 | ||
| 881 | + should 'filter fields with white_list filter' do | ||
| 882 | + environment = Environment.new | ||
| 883 | + environment.message_for_disabled_enterprise = "<h1> Disabled Enterprise </h1>" | ||
| 884 | + environment.valid? | ||
| 885 | + | ||
| 886 | + assert_equal "<h1> Disabled Enterprise </h1>", environment.message_for_disabled_enterprise | ||
| 887 | + end | ||
| 888 | + | ||
| 889 | + should 'escape malformed html tags' do | ||
| 890 | + environment = Environment.new | ||
| 891 | + environment.message_for_disabled_enterprise = "<h1> Disabled Enterprise /h1>" | ||
| 892 | + environment.valid? | ||
| 893 | + | ||
| 894 | + assert_no_match /[<>]/, environment.message_for_disabled_enterprise | ||
| 895 | + end | ||
| 896 | + | ||
| 881 | end | 897 | end |
test/unit/event_test.rb
| @@ -222,4 +222,32 @@ class EventTest < ActiveSupport::TestCase | @@ -222,4 +222,32 @@ class EventTest < ActiveSupport::TestCase | ||
| 222 | assert_not_includes profile.events.by_day(today), event_out_of_range | 222 | assert_not_includes profile.events.by_day(today), event_out_of_range |
| 223 | end | 223 | end |
| 224 | 224 | ||
| 225 | + should 'filter fields with full filter' do | ||
| 226 | + event = Event.new | ||
| 227 | + event.link = "<h1 Malformed >> html >< tag" | ||
| 228 | + event.valid? | ||
| 229 | + | ||
| 230 | + assert_no_match /[<>]/, event.link | ||
| 231 | + end | ||
| 232 | + | ||
| 233 | + should 'filter fields with white_list filter' do | ||
| 234 | + event = Event.new | ||
| 235 | + event.description = "<h1> Description </h1>" | ||
| 236 | + event.address = "<strong> Address <strong>" | ||
| 237 | + event.valid? | ||
| 238 | + | ||
| 239 | + assert_equal "<h1> Description </h1>", event.description | ||
| 240 | + assert_equal "<strong> Address <strong>", event.address | ||
| 241 | + end | ||
| 242 | + | ||
| 243 | + should 'escape malformed html tags' do | ||
| 244 | + event = Event.new | ||
| 245 | + event.description = "<h1<< Description >>/h1>" | ||
| 246 | + event.address = "<strong>><< Address <strong>" | ||
| 247 | + event.valid? | ||
| 248 | + | ||
| 249 | + assert_no_match /[<>]/, event.description | ||
| 250 | + assert_no_match /[<>]/, event.address | ||
| 251 | + end | ||
| 252 | + | ||
| 225 | end | 253 | end |
test/unit/folder_test.rb
| @@ -125,17 +125,27 @@ class FolderTest < ActiveSupport::TestCase | @@ -125,17 +125,27 @@ class FolderTest < ActiveSupport::TestCase | ||
| 125 | end | 125 | end |
| 126 | 126 | ||
| 127 | should 'not let pass javascript in the body' do | 127 | should 'not let pass javascript in the body' do |
| 128 | - owner = create_user('testuser').person | ||
| 129 | - folder = fast_create(Folder, {:profile_id => owner.id, :body => '<script>alert("Xss Attack!")</script>'}) | ||
| 130 | - folder.save! | ||
| 131 | - assert_no_match(/<script>/, folder.body) | 128 | + folder = Folder.new |
| 129 | + folder.body = "<script> alert(Xss!); </script>" | ||
| 130 | + folder.valid? | ||
| 131 | + | ||
| 132 | + assert_no_match /(<script>)/, folder.body | ||
| 132 | end | 133 | end |
| 133 | 134 | ||
| 134 | - should 'let pass html in the body' do | ||
| 135 | - owner = create_user('testuser').person | ||
| 136 | - folder = fast_create(Folder, {:profile_id => owner.id, :body => '<strong>I am not a Xss Attack!")</strong>'}) | ||
| 137 | - folder.save! | ||
| 138 | - assert_match(/<strong>/, folder.body) | 135 | + should 'filter fields with white_list filter' do |
| 136 | + folder = Folder.new | ||
| 137 | + folder.body = "<h1> Body </h1>" | ||
| 138 | + folder.valid? | ||
| 139 | + | ||
| 140 | + assert_equal "<h1> Body </h1>", folder.body | ||
| 141 | + end | ||
| 142 | + | ||
| 143 | + should 'escape malformed html tags' do | ||
| 144 | + folder = Folder.new | ||
| 145 | + folder.body = "<h1<< Description >>/h1>" | ||
| 146 | + folder.valid? | ||
| 147 | + | ||
| 148 | + assert_no_match /[<>]/, folder.body | ||
| 139 | end | 149 | end |
| 140 | 150 | ||
| 141 | end | 151 | end |
test/unit/organization_test.rb
| @@ -248,4 +248,24 @@ class OrganizationTest < Test::Unit::TestCase | @@ -248,4 +248,24 @@ class OrganizationTest < Test::Unit::TestCase | ||
| 248 | 248 | ||
| 249 | assert organization.closed | 249 | assert organization.closed |
| 250 | end | 250 | end |
| 251 | + | ||
| 252 | + should 'escape malformed html tags' do | ||
| 253 | + organization = Organization.new | ||
| 254 | + organization.acronym = "<h1 Malformed >> html >< tag" | ||
| 255 | + organization.contact_person = "<h1 Malformed >,<<<asfdf> html >< tag" | ||
| 256 | + organization.contact_email = "<h1<malformed@html.com>>" | ||
| 257 | + organization.description = "<h1 Malformed /h1>>><<> html ><>h1< tag" | ||
| 258 | + organization.legal_form = "<h1 Malformed /h1>>><<> html ><>h1< tag" | ||
| 259 | + organization.economic_activity = "<h1 Malformed /h1>>><<> html ><>h1< tag" | ||
| 260 | + organization.management_information = "<h1 Malformed /h1>>><<> html ><>h1< tag" | ||
| 261 | + organization.valid? | ||
| 262 | + | ||
| 263 | + assert_no_match /[<>]/, organization.acronym | ||
| 264 | + assert_no_match /[<>]/, organization.contact_person | ||
| 265 | + assert_no_match /[<>]/, organization.contact_email | ||
| 266 | + assert_no_match /[<>]/, organization.legal_form | ||
| 267 | + assert_no_match /[<>]/, organization.economic_activity | ||
| 268 | + assert_no_match /[<>]/, organization.management_information | ||
| 269 | + end | ||
| 270 | + | ||
| 251 | end | 271 | end |
test/unit/product_test.rb
| @@ -193,4 +193,22 @@ class ProductTest < Test::Unit::TestCase | @@ -193,4 +193,22 @@ class ProductTest < Test::Unit::TestCase | ||
| 193 | end | 193 | end |
| 194 | end | 194 | end |
| 195 | 195 | ||
| 196 | + should 'sanitize name before validation' do | ||
| 197 | + product = Product.new | ||
| 198 | + product.name = "<h1 Bla </h1>" | ||
| 199 | + product.valid? | ||
| 200 | + | ||
| 201 | + assert product.errors.invalid?(:name) | ||
| 202 | + end | ||
| 203 | + | ||
| 204 | + should 'escape malformed html tags' do | ||
| 205 | + product = Product.new | ||
| 206 | + product.name = "<h1 Malformed >> html >< tag" | ||
| 207 | + product.description = "<h1 Malformed</h1>><<<a>> >> html >< tag" | ||
| 208 | + product.valid? | ||
| 209 | + | ||
| 210 | + assert_no_match /[<>]/, product.name | ||
| 211 | + assert_no_match /[<>]/, product.description | ||
| 212 | + end | ||
| 213 | + | ||
| 196 | end | 214 | end |
test/unit/profile_test.rb
| @@ -1515,6 +1515,44 @@ class ProfileTest < Test::Unit::TestCase | @@ -1515,6 +1515,44 @@ class ProfileTest < Test::Unit::TestCase | ||
| 1515 | assert profile.errors.invalid?(:description) | 1515 | assert profile.errors.invalid?(:description) |
| 1516 | end | 1516 | end |
| 1517 | 1517 | ||
| 1518 | + should 'sanitize name before validation' do | ||
| 1519 | + profile = Profile.new | ||
| 1520 | + profile.name = "<h1 Bla </h1>" | ||
| 1521 | + profile.valid? | ||
| 1522 | + | ||
| 1523 | + assert profile.errors.invalid?(:name) | ||
| 1524 | + end | ||
| 1525 | + | ||
| 1526 | + should 'filter fields with white_list filter' do | ||
| 1527 | + profile = Profile.new | ||
| 1528 | + profile.custom_header = "<h1> Custom Header </h1>" | ||
| 1529 | + profile.custom_footer = "<strong> Custom Footer <strong>" | ||
| 1530 | + profile.valid? | ||
| 1531 | + | ||
| 1532 | + assert_equal "<h1> Custom Header </h1>", profile.custom_header | ||
| 1533 | + assert_equal "<strong> Custom Footer <strong>", profile.custom_footer | ||
| 1534 | + end | ||
| 1535 | + | ||
| 1536 | + should 'escape malformed html tags' do | ||
| 1537 | + profile = Profile.new | ||
| 1538 | + profile.name = "<h1 Malformed >> html >>></a>< tag" | ||
| 1539 | + profile.nickname = "<h1 Malformed <<h1>>< html >< tag" | ||
| 1540 | + profile.address = "<h1><</h2< Malformed >> html >< tag" | ||
| 1541 | + profile.contact_phone = "<h1<< Malformed ><>>> html >< tag" | ||
| 1542 | + profile.description = "<h1<a> Malformed >> html ></a>< tag" | ||
| 1543 | + profile.custom_header = "<h1<a>><<> Malformed >> html ></a>< tag" | ||
| 1544 | + profile.custom_footer = "<h1> Malformed <><< html ></a>< tag" | ||
| 1545 | + profile.valid? | ||
| 1546 | + | ||
| 1547 | + assert_no_match /[<>]/, profile.name | ||
| 1548 | + assert_no_match /[<>]/, profile.nickname | ||
| 1549 | + assert_no_match /[<>]/, profile.address | ||
| 1550 | + assert_no_match /[<>]/, profile.contact_phone | ||
| 1551 | + assert_no_match /[<>]/, profile.description | ||
| 1552 | + assert_no_match /[<>]/, profile.custom_header | ||
| 1553 | + assert_no_match /[<>]/, profile.custom_footer | ||
| 1554 | + end | ||
| 1555 | + | ||
| 1518 | private | 1556 | private |
| 1519 | 1557 | ||
| 1520 | def assert_invalid_identifier(id) | 1558 | def assert_invalid_identifier(id) |
test/unit/text_article_test.rb
| @@ -26,4 +26,17 @@ class TextArticleTest < Test::Unit::TestCase | @@ -26,4 +26,17 @@ class TextArticleTest < Test::Unit::TestCase | ||
| 26 | assert_equal "the article ...", article.body | 26 | assert_equal "the article ...", article.body |
| 27 | end | 27 | end |
| 28 | 28 | ||
| 29 | + should 'escape malformed html tags' do | ||
| 30 | + person = create_user('testuser').person | ||
| 31 | + article = TextArticle.new(:profile => person) | ||
| 32 | + article.name = "<h1 Malformed >> html >>></a>< tag" | ||
| 33 | + article.abstract = "<h1 Malformed <<h1>>< html >< tag" | ||
| 34 | + article.body = "<h1><</h2< Malformed >> html >< tag" | ||
| 35 | + article.valid? | ||
| 36 | + | ||
| 37 | + assert_no_match /[<>]/, article.name | ||
| 38 | + assert_no_match /[<>]/, article.abstract | ||
| 39 | + assert_no_match /[<>]/, article.body | ||
| 40 | + end | ||
| 41 | + | ||
| 29 | end | 42 | end |
test/unit/validation_info_test.rb
| @@ -21,4 +21,14 @@ class ValidationInfoTest < Test::Unit::TestCase | @@ -21,4 +21,14 @@ class ValidationInfoTest < Test::Unit::TestCase | ||
| 21 | end | 21 | end |
| 22 | end | 22 | end |
| 23 | 23 | ||
| 24 | + should 'escape malformed html tags' do | ||
| 25 | + info = ValidationInfo.new | ||
| 26 | + info.validation_methodology = "<h1 Malformed >> html >< tag" | ||
| 27 | + info.restrictions = "<h1 Malformed >> html >< tag" | ||
| 28 | + info.valid? | ||
| 29 | + | ||
| 30 | + assert_no_match /[<>]/, info.validation_methodology | ||
| 31 | + assert_no_match /[<>]/, info.restrictions | ||
| 32 | + end | ||
| 33 | + | ||
| 24 | end | 34 | end |
vendor/plugins/xss_terminate/lib/xss_terminate.rb
| @@ -38,7 +38,7 @@ module XssTerminate | @@ -38,7 +38,7 @@ module XssTerminate | ||
| 38 | 38 | ||
| 39 | module InstanceMethods | 39 | module InstanceMethods |
| 40 | 40 | ||
| 41 | - def sanitize_field(sanitizer, field, serialized = false) | 41 | + def sanitize_field(sanitizer, field, serialized = false, with= :full) |
| 42 | field = field.to_sym | 42 | field = field.to_sym |
| 43 | if serialized | 43 | if serialized |
| 44 | puts field | 44 | puts field |
| @@ -49,8 +49,22 @@ module XssTerminate | @@ -49,8 +49,22 @@ module XssTerminate | ||
| 49 | else | 49 | else |
| 50 | if self[field] | 50 | if self[field] |
| 51 | self[field] = sanitizer.sanitize(self[field]) | 51 | self[field] = sanitizer.sanitize(self[field]) |
| 52 | + | ||
| 53 | + if with == :full | ||
| 54 | + self[field] = CGI.escapeHTML(self[field]) | ||
| 55 | + elsif with == :white_list | ||
| 56 | + self[field] = CGI.escapeHTML(self[field]) if !wellformed_html_tag?(self[field]) | ||
| 57 | + end | ||
| 58 | + | ||
| 52 | else | 59 | else |
| 53 | self.send("#{field}=", sanitizer.sanitize(self.send("#{field}"))) | 60 | self.send("#{field}=", sanitizer.sanitize(self.send("#{field}"))) |
| 61 | + | ||
| 62 | + if with == :full | ||
| 63 | + self.send("#{field}=", CGI.escapeHTML(self.send("#{field}"))) | ||
| 64 | + elsif with == :white_list | ||
| 65 | + self.send("#{field}=", CGI.escapeHTML(self.send("#{field}"))) if !wellformed_html_tag?(self.send("#{field}")) | ||
| 66 | + end | ||
| 67 | + | ||
| 54 | end | 68 | end |
| 55 | end | 69 | end |
| 56 | end | 70 | end |
| @@ -69,7 +83,7 @@ module XssTerminate | @@ -69,7 +83,7 @@ module XssTerminate | ||
| 69 | sanitizer = RailsSanitize.full_sanitizer | 83 | sanitizer = RailsSanitize.full_sanitizer |
| 70 | columns, columns_serialized = sanitize_columns(:full) | 84 | columns, columns_serialized = sanitize_columns(:full) |
| 71 | columns.each do |column| | 85 | columns.each do |column| |
| 72 | - sanitize_field(sanitizer, column.to_sym, columns_serialized.include?(column)) | 86 | + sanitize_field(sanitizer, column.to_sym, columns_serialized.include?(column), :full) |
| 73 | end | 87 | end |
| 74 | end | 88 | end |
| 75 | 89 | ||
| @@ -77,18 +91,33 @@ module XssTerminate | @@ -77,18 +91,33 @@ module XssTerminate | ||
| 77 | sanitizer = RailsSanitize.white_list_sanitizer | 91 | sanitizer = RailsSanitize.white_list_sanitizer |
| 78 | columns, columns_serialized = sanitize_columns(:white_list) | 92 | columns, columns_serialized = sanitize_columns(:white_list) |
| 79 | columns.each do |column| | 93 | columns.each do |column| |
| 80 | - sanitize_field(sanitizer, column.to_sym, columns_serialized.include?(column)) | 94 | + sanitize_field(sanitizer, column.to_sym, columns_serialized.include?(column), :white_list) |
| 81 | end | 95 | end |
| 82 | - end | 96 | + end |
| 83 | 97 | ||
| 84 | def sanitize_fields_with_html5lib | 98 | def sanitize_fields_with_html5lib |
| 85 | sanitizer = HTML5libSanitize.new | 99 | sanitizer = HTML5libSanitize.new |
| 86 | columns = sanitize_columns(:html5lib) | 100 | columns = sanitize_columns(:html5lib) |
| 87 | columns.each do |column| | 101 | columns.each do |column| |
| 88 | - sanitize_field(sanitizer, column.to_sym, columns_serialized.include?(column)) | 102 | + sanitize_field(sanitizer, column.to_sym, columns_serialized.include?(column), :html5lib) |
| 89 | end | 103 | end |
| 90 | end | 104 | end |
| 91 | 105 | ||
| 106 | + def wellformed_html_tag?(field) | ||
| 107 | + return true if !field | ||
| 108 | + | ||
| 109 | + counter = 0 | ||
| 110 | + field.split(//).each do |letter| | ||
| 111 | + counter += 1 if letter == '<' | ||
| 112 | + counter -= 1 if letter == '>' | ||
| 113 | + if counter < 0 || 1 < counter | ||
| 114 | + return false | ||
| 115 | + end | ||
| 116 | + end | ||
| 117 | + | ||
| 118 | + return counter == 0 | ||
| 119 | + end | ||
| 120 | + | ||
| 92 | end | 121 | end |
| 93 | 122 | ||
| 94 | end | 123 | end |