Commit 688faaee41448c11504adb3e94ab4d63413b7ffa
Committed by
Antonio Terceiro
1 parent
52b58187
Exists in
master
and in
29 other branches
Fields verification filters malformed html correctly
* The field verification now occurs before de validation. * Fields that uses the full filter escape residual '<' and '>' after filtering wellformed tags. * Fields that uses white_list filter only escape the '<' and '>' symbols if the tag is not wellformed. * This patch only fixes the fields that were before being filtered. A full field verification is need to check the way each field must be filtered. (ActionItem1491)
Showing
26 changed files
with
273 additions
and
33 deletions
Show diff stats
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 |
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
@@ -863,4 +863,20 @@ class EnvironmentTest < Test::Unit::TestCase | @@ -863,4 +863,20 @@ class EnvironmentTest < Test::Unit::TestCase | ||
863 | assert_equal env.message_for_member_invitation, env.invitation_mail_template(community) | 863 | assert_equal env.message_for_member_invitation, env.invitation_mail_template(community) |
864 | end | 864 | end |
865 | 865 | ||
866 | + should 'filter fields with white_list filter' do | ||
867 | + environment = Environment.new | ||
868 | + environment.message_for_disabled_enterprise = "<h1> Disabled Enterprise </h1>" | ||
869 | + environment.valid? | ||
870 | + | ||
871 | + assert_equal "<h1> Disabled Enterprise </h1>", environment.message_for_disabled_enterprise | ||
872 | + end | ||
873 | + | ||
874 | + should 'escape malformed html tags' do | ||
875 | + environment = Environment.new | ||
876 | + environment.message_for_disabled_enterprise = "<h1> Disabled Enterprise /h1>" | ||
877 | + environment.valid? | ||
878 | + | ||
879 | + assert_no_match /[<>]/, environment.message_for_disabled_enterprise | ||
880 | + end | ||
881 | + | ||
866 | end | 882 | 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 |