Commit 5640070915e3b18a29e3a941dc96a603529fb40c
1 parent
89b2559c
Exists in
master
and in
29 other branches
avoid html double escape
This is already handled on rails 3. Fix #62
Showing
13 changed files
with
16 additions
and
209 deletions
Show diff stats
test/unit/article_test.rb
@@ -931,14 +931,6 @@ class ArticleTest < ActiveSupport::TestCase | @@ -931,14 +931,6 @@ class ArticleTest < ActiveSupport::TestCase | ||
931 | assert_no_match(/<script>/, a.name) | 931 | assert_no_match(/<script>/, a.name) |
932 | end | 932 | end |
933 | 933 | ||
934 | - should 'escape malformed html tags' do | ||
935 | - article = Article.new | ||
936 | - article.name = "<h1 Malformed >> html >< tag" | ||
937 | - article.valid? | ||
938 | - | ||
939 | - assert_no_match /[<>]/, article.name | ||
940 | - end | ||
941 | - | ||
942 | should 'return truncated title in short_title' do | 934 | should 'return truncated title in short_title' do |
943 | article = Article.new | 935 | article = Article.new |
944 | article.name = 'a123456789abcdefghij' | 936 | article.name = 'a123456789abcdefghij' |
test/unit/comment_test.rb
@@ -201,17 +201,6 @@ class CommentTest < ActiveSupport::TestCase | @@ -201,17 +201,6 @@ class CommentTest < ActiveSupport::TestCase | ||
201 | assert comment.errors[:body.to_s].present? | 201 | assert comment.errors[:body.to_s].present? |
202 | end | 202 | end |
203 | 203 | ||
204 | - should 'escape malformed html tags' do | ||
205 | - owner = create_user('testuser').person | ||
206 | - article = owner.articles.create(:name => 'test', :body => '...') | ||
207 | - comment = build(Comment, :article => article, :title => '<h1 title </h1>>> sd f <<', :body => '<h1>> sdf><asd>< body </h1>', :name => '<h1 name </h1>>><<dfsf<sd', :email => 'cracker@test.org') | ||
208 | - comment.valid? | ||
209 | - | ||
210 | - assert_no_match /[<>]/, comment.title | ||
211 | - assert_no_match /[<>]/, comment.body | ||
212 | - assert_no_match /[<>]/, comment.name | ||
213 | - end | ||
214 | - | ||
215 | should 'use an existing image for deleted comments' do | 204 | should 'use an existing image for deleted comments' do |
216 | image = Comment.new.removed_user_image[1..-1] | 205 | image = Comment.new.removed_user_image[1..-1] |
217 | assert File.exists?(Rails.root.join('public', image)), "#{image} does not exist." | 206 | assert File.exists?(Rails.root.join('public', image)), "#{image} does not exist." |
@@ -749,6 +738,18 @@ class CommentTest < ActiveSupport::TestCase | @@ -749,6 +738,18 @@ class CommentTest < ActiveSupport::TestCase | ||
749 | comment.destroy | 738 | comment.destroy |
750 | end | 739 | end |
751 | 740 | ||
741 | + should 'not double escape html content after validation' do | ||
742 | + comment = create_comment | ||
743 | + body = 'Comment with "quotes"' | ||
744 | + comment.body = body | ||
745 | + | ||
746 | + comment.valid? | ||
747 | + assert_equal body, comment.body | ||
748 | + | ||
749 | + comment.valid? | ||
750 | + assert_equal body, comment.body | ||
751 | + end | ||
752 | + | ||
752 | private | 753 | private |
753 | 754 | ||
754 | def create_comment(args = {}) | 755 | def create_comment(args = {}) |
test/unit/community_test.rb
@@ -242,20 +242,6 @@ class CommunityTest < ActiveSupport::TestCase | @@ -242,20 +242,6 @@ class CommunityTest < ActiveSupport::TestCase | ||
242 | end | 242 | end |
243 | end | 243 | end |
244 | 244 | ||
245 | - should 'escape malformed html tags' do | ||
246 | - community = Community.new | ||
247 | - community.name = "<h1 Malformed >> html >< tag" | ||
248 | - community.address = "<h1 Malformed >,<<<asfdf> html >< tag" | ||
249 | - community.contact_phone = "<h1 Malformed<<> >> html >><>< tag" | ||
250 | - community.description = "<h1 Malformed /h1>>><<> html ><>h1< tag" | ||
251 | - community.valid? | ||
252 | - | ||
253 | - assert_no_match /[<>]/, community.name | ||
254 | - assert_no_match /[<>]/, community.address | ||
255 | - assert_no_match /[<>]/, community.contact_phone | ||
256 | - assert_no_match /[<>]/, community.description | ||
257 | - end | ||
258 | - | ||
259 | should "the followed_by method be protected and true to the community members by default" do | 245 | should "the followed_by method be protected and true to the community members by default" do |
260 | c = fast_create(Community) | 246 | c = fast_create(Community) |
261 | p1 = fast_create(Person) | 247 | p1 = fast_create(Person) |
test/unit/environment_test.rb
@@ -1140,14 +1140,6 @@ class EnvironmentTest < ActiveSupport::TestCase | @@ -1140,14 +1140,6 @@ class EnvironmentTest < ActiveSupport::TestCase | ||
1140 | assert_equal "<h1> Disabled Enterprise </h1>", environment.message_for_disabled_enterprise | 1140 | assert_equal "<h1> Disabled Enterprise </h1>", environment.message_for_disabled_enterprise |
1141 | end | 1141 | end |
1142 | 1142 | ||
1143 | - should 'escape malformed html tags' do | ||
1144 | - environment = Environment.new | ||
1145 | - environment.message_for_disabled_enterprise = "<h1> Disabled Enterprise /h1>" | ||
1146 | - environment.valid? | ||
1147 | - | ||
1148 | - assert_no_match /[<>]/, environment.message_for_disabled_enterprise | ||
1149 | - end | ||
1150 | - | ||
1151 | should 'not sanitize html comments' do | 1143 | should 'not sanitize html comments' do |
1152 | environment = Environment.new | 1144 | environment = Environment.new |
1153 | environment.message_for_disabled_enterprise = '<p><!-- <asdf> << aasdfa >>> --> <h1> Wellformed html code </h1>' | 1145 | environment.message_for_disabled_enterprise = '<p><!-- <asdf> << aasdfa >>> --> <h1> Wellformed html code </h1>' |
test/unit/event_test.rb
@@ -263,24 +263,6 @@ class EventTest < ActiveSupport::TestCase | @@ -263,24 +263,6 @@ class EventTest < ActiveSupport::TestCase | ||
263 | assert_not_includes profile.events.by_day(today), event_out_of_range | 263 | assert_not_includes profile.events.by_day(today), event_out_of_range |
264 | end | 264 | end |
265 | 265 | ||
266 | - should 'filter fields with full filter' do | ||
267 | - event = Event.new | ||
268 | - event.link = "<h1 Malformed >> html >< tag" | ||
269 | - event.valid? | ||
270 | - | ||
271 | - assert_no_match /[<>]/, event.link | ||
272 | - end | ||
273 | - | ||
274 | - should 'filter fields with white_list filter' do | ||
275 | - event = Event.new | ||
276 | - event.body = "<h1> Description </h1>" | ||
277 | - event.address = "<strong> Address <strong>" | ||
278 | - event.valid? | ||
279 | - | ||
280 | - assert_equal "<h1> Description </h1>", event.body | ||
281 | - assert_equal "<strong> Address <strong>", event.address | ||
282 | - end | ||
283 | - | ||
284 | should 'not filter & on link field' do | 266 | should 'not filter & on link field' do |
285 | event = Event.new | 267 | event = Event.new |
286 | event.link = 'myevent.com/?param1=value¶m2=value2' | 268 | event.link = 'myevent.com/?param1=value¶m2=value2' |
@@ -289,16 +271,6 @@ class EventTest < ActiveSupport::TestCase | @@ -289,16 +271,6 @@ class EventTest < ActiveSupport::TestCase | ||
289 | assert_equal "http://myevent.com/?param1=value¶m2=value2", event.link | 271 | assert_equal "http://myevent.com/?param1=value¶m2=value2", event.link |
290 | end | 272 | end |
291 | 273 | ||
292 | - should 'escape malformed html tags' do | ||
293 | - event = Event.new | ||
294 | - event.body = "<h1<< Description >>/h1>" | ||
295 | - event.address = "<strong>><< Address <strong>" | ||
296 | - event.valid? | ||
297 | - | ||
298 | - assert_no_match /[<>]/, event.body | ||
299 | - assert_no_match /[<>]/, event.address | ||
300 | - end | ||
301 | - | ||
302 | should 'not sanitize html comments' do | 274 | should 'not sanitize html comments' do |
303 | event = Event.new | 275 | event = Event.new |
304 | event.body = '<p><!-- <asdf> << aasdfa >>> --> <h1> Wellformed html code </h1>' | 276 | event.body = '<p><!-- <asdf> << aasdfa >>> --> <h1> Wellformed html code </h1>' |
test/unit/folder_test.rb
@@ -133,14 +133,6 @@ class FolderTest < ActiveSupport::TestCase | @@ -133,14 +133,6 @@ class FolderTest < ActiveSupport::TestCase | ||
133 | assert_match /<!-- .* --> <h1> Wellformed html code <\/h1>/, folder.body | 133 | assert_match /<!-- .* --> <h1> Wellformed html code <\/h1>/, folder.body |
134 | end | 134 | end |
135 | 135 | ||
136 | - should 'escape malformed html tags' do | ||
137 | - folder = Folder.new | ||
138 | - folder.body = "<h1<< Description >>/h1>" | ||
139 | - folder.valid? | ||
140 | - | ||
141 | - assert_no_match /[<>]/, folder.body | ||
142 | - end | ||
143 | - | ||
144 | should 'not have a blog as parent' do | 136 | should 'not have a blog as parent' do |
145 | folder = Folder.new | 137 | folder = Folder.new |
146 | folder.parent = Blog.new | 138 | folder.parent = Blog.new |
test/unit/gallery_test.rb
@@ -134,14 +134,6 @@ class GalleryTest < ActiveSupport::TestCase | @@ -134,14 +134,6 @@ class GalleryTest < ActiveSupport::TestCase | ||
134 | assert_match /<!-- .* --> <h1> Wellformed html code <\/h1>/, gallery.body | 134 | assert_match /<!-- .* --> <h1> Wellformed html code <\/h1>/, gallery.body |
135 | end | 135 | end |
136 | 136 | ||
137 | - should 'escape malformed html tags' do | ||
138 | - gallery = Gallery.new | ||
139 | - gallery.body = "<h1<< Description >>/h1>" | ||
140 | - gallery.valid? | ||
141 | - | ||
142 | - assert_no_match /[<>]/, gallery.body | ||
143 | - end | ||
144 | - | ||
145 | should 'accept uploads' do | 137 | should 'accept uploads' do |
146 | folder = fast_create(Gallery) | 138 | folder = fast_create(Gallery) |
147 | assert folder.accept_uploads? | 139 | assert folder.accept_uploads? |
test/unit/organization_test.rb
@@ -253,25 +253,6 @@ class OrganizationTest < ActiveSupport::TestCase | @@ -253,25 +253,6 @@ class OrganizationTest < ActiveSupport::TestCase | ||
253 | assert organization.closed | 253 | assert organization.closed |
254 | end | 254 | end |
255 | 255 | ||
256 | - should 'escape malformed html tags' do | ||
257 | - organization = Organization.new | ||
258 | - organization.acronym = "<h1 Malformed >> html >< tag" | ||
259 | - organization.contact_person = "<h1 Malformed >,<<<asfdf> html >< tag" | ||
260 | - organization.contact_email = "<h1<malformed@html.com>>" | ||
261 | - organization.description = "<h1 Malformed /h1>>><<> html ><>h1< tag" | ||
262 | - organization.legal_form = "<h1 Malformed /h1>>><<> html ><>h1< tag" | ||
263 | - organization.economic_activity = "<h1 Malformed /h1>>><<> html ><>h1< tag" | ||
264 | - organization.management_information = "<h1 Malformed /h1>>><<> html ><>h1< tag" | ||
265 | - organization.valid? | ||
266 | - | ||
267 | - assert_no_match /[<>]/, organization.acronym | ||
268 | - assert_no_match /[<>]/, organization.contact_person | ||
269 | - assert_no_match /[<>]/, organization.contact_email | ||
270 | - assert_no_match /[<>]/, organization.legal_form | ||
271 | - assert_no_match /[<>]/, organization.economic_activity | ||
272 | - assert_no_match /[<>]/, organization.management_information | ||
273 | - end | ||
274 | - | ||
275 | should "the followed_by? be true only to members" do | 256 | should "the followed_by? be true only to members" do |
276 | o = fast_create(Organization) | 257 | o = fast_create(Organization) |
277 | p1 = fast_create(Person) | 258 | p1 = fast_create(Person) |
test/unit/product_test.rb
@@ -171,16 +171,6 @@ class ProductTest < ActiveSupport::TestCase | @@ -171,16 +171,6 @@ class ProductTest < ActiveSupport::TestCase | ||
171 | assert_equal @product_category.name, product.name | 171 | assert_equal @product_category.name, product.name |
172 | end | 172 | end |
173 | 173 | ||
174 | - should 'escape malformed html tags' do | ||
175 | - product = build(Product, :product_category => @product_category) | ||
176 | - product.name = "<h1 Malformed >> html >< tag" | ||
177 | - product.description = "<h1 Malformed</h1>><<<a>> >> html >< tag" | ||
178 | - product.valid? | ||
179 | - | ||
180 | - assert_no_match /[<>]/, product.name | ||
181 | - assert_no_match /[<>]/, product.description | ||
182 | - end | ||
183 | - | ||
184 | should 'use name of category when has no name yet' do | 174 | should 'use name of category when has no name yet' do |
185 | product = Product.new | 175 | product = Product.new |
186 | product.product_category = @product_category | 176 | product.product_category = @product_category |
test/unit/profile_test.rb
@@ -1696,34 +1696,6 @@ class ProfileTest < ActiveSupport::TestCase | @@ -1696,34 +1696,6 @@ class ProfileTest < ActiveSupport::TestCase | ||
1696 | assert_equal "<strong> Custom Footer <strong>", profile.custom_footer | 1696 | assert_equal "<strong> Custom Footer <strong>", profile.custom_footer |
1697 | end | 1697 | end |
1698 | 1698 | ||
1699 | - should 'escape malformed html tags' do | ||
1700 | - profile = Profile.new | ||
1701 | - profile.name = "<h1 Malformed >> html >>></a>< tag" | ||
1702 | - profile.nickname = "<h1 Malformed <<h1>>< html >< tag" | ||
1703 | - profile.address = "<h1><</h2< Malformed >> html >< tag" | ||
1704 | - profile.contact_phone = "<h1<< Malformed ><>>> html >< tag" | ||
1705 | - profile.description = "<h1<a> Malformed >> html ></a>< tag" | ||
1706 | - profile.valid? | ||
1707 | - | ||
1708 | - assert_no_match /[<>]/, profile.name | ||
1709 | - assert_no_match /[<>]/, profile.nickname | ||
1710 | - assert_no_match /[<>]/, profile.address | ||
1711 | - assert_no_match /[<>]/, profile.contact_phone | ||
1712 | - assert_no_match /[<>]/, profile.description | ||
1713 | - assert_no_match /[<>]/, profile.custom_header | ||
1714 | - assert_no_match /[<>]/, profile.custom_footer | ||
1715 | - end | ||
1716 | - | ||
1717 | - should 'escape malformed html tags in header and footer' do | ||
1718 | - profile = fast_create(Profile) | ||
1719 | - profile.custom_header = "<h1<a>><<> Malformed >> html ></a>< tag" | ||
1720 | - profile.custom_footer = "<h1> Malformed <><< html ></a>< tag" | ||
1721 | - profile.save | ||
1722 | - | ||
1723 | - assert_no_match /[<>]/, profile.custom_header | ||
1724 | - assert_no_match /[<>]/, profile.custom_footer | ||
1725 | - end | ||
1726 | - | ||
1727 | should 'not sanitize html comments' do | 1699 | should 'not sanitize html comments' do |
1728 | profile = Profile.new | 1700 | profile = Profile.new |
1729 | profile.custom_header = '<p><!-- <asdf> << aasdfa >>> --> <h1> Wellformed html code </h1>' | 1701 | profile.custom_header = '<p><!-- <asdf> << aasdfa >>> --> <h1> Wellformed html code </h1>' |
test/unit/text_article_test.rb
@@ -14,15 +14,6 @@ class TextArticleTest < ActiveSupport::TestCase | @@ -14,15 +14,6 @@ class TextArticleTest < ActiveSupport::TestCase | ||
14 | assert_includes TextArticle.find(:all), article | 14 | assert_includes TextArticle.find(:all), article |
15 | end | 15 | end |
16 | 16 | ||
17 | - should 'remove HTML from name' do | ||
18 | - person = create_user('testuser').person | ||
19 | - article = TextArticle.new(:profile => person) | ||
20 | - article.name = "<h1 Malformed >> html >>></a>< tag" | ||
21 | - article.valid? | ||
22 | - | ||
23 | - assert_no_match /[<>]/, article.name | ||
24 | - end | ||
25 | - | ||
26 | should 'be translatable' do | 17 | should 'be translatable' do |
27 | assert_kind_of Noosfero::TranslatableContent, TextArticle.new | 18 | assert_kind_of Noosfero::TranslatableContent, TextArticle.new |
28 | end | 19 | end |
test/unit/validation_info_test.rb
@@ -21,14 +21,4 @@ class ValidationInfoTest < ActiveSupport::TestCase | @@ -21,14 +21,4 @@ class ValidationInfoTest < ActiveSupport::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 | - | ||
34 | end | 24 | 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, with= :full) | 41 | + def sanitize_field(sanitizer, field, serialized = false) |
42 | field = field.to_sym | 42 | field = field.to_sym |
43 | if serialized | 43 | if serialized |
44 | puts field | 44 | puts field |
@@ -49,25 +49,11 @@ module XssTerminate | @@ -49,25 +49,11 @@ 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_code?(self[field]) | ||
57 | - end | ||
58 | - | ||
59 | else | 52 | else |
60 | value = self.send("#{field}") | 53 | value = self.send("#{field}") |
61 | return unless value | 54 | return unless value |
62 | value = sanitizer.sanitize(value) | 55 | value = sanitizer.sanitize(value) |
63 | self.send("#{field}=", value) | 56 | self.send("#{field}=", value) |
64 | - | ||
65 | - if with == :full | ||
66 | - self.send("#{field}=", CGI.escapeHTML(value)) | ||
67 | - elsif with == :white_list | ||
68 | - self.send("#{field}=", CGI.escapeHTML(value)) if !wellformed_html_code?(value) | ||
69 | - end | ||
70 | - | ||
71 | end | 57 | end |
72 | end | 58 | end |
73 | end | 59 | end |
@@ -86,7 +72,7 @@ module XssTerminate | @@ -86,7 +72,7 @@ module XssTerminate | ||
86 | sanitizer = ActionView::Base.full_sanitizer | 72 | sanitizer = ActionView::Base.full_sanitizer |
87 | columns, columns_serialized = sanitize_columns(:full) | 73 | columns, columns_serialized = sanitize_columns(:full) |
88 | columns.each do |column| | 74 | columns.each do |column| |
89 | - sanitize_field(sanitizer, column.to_sym, columns_serialized.include?(column), :full) | 75 | + sanitize_field(sanitizer, column.to_sym, columns_serialized.include?(column)) |
90 | end | 76 | end |
91 | end | 77 | end |
92 | 78 | ||
@@ -94,7 +80,7 @@ module XssTerminate | @@ -94,7 +80,7 @@ module XssTerminate | ||
94 | sanitizer = ActionView::Base.white_list_sanitizer | 80 | sanitizer = ActionView::Base.white_list_sanitizer |
95 | columns, columns_serialized = sanitize_columns(:white_list) | 81 | columns, columns_serialized = sanitize_columns(:white_list) |
96 | columns.each do |column| | 82 | columns.each do |column| |
97 | - sanitize_field(sanitizer, column.to_sym, columns_serialized.include?(column), :white_list) | 83 | + sanitize_field(sanitizer, column.to_sym, columns_serialized.include?(column)) |
98 | end | 84 | end |
99 | end | 85 | end |
100 | 86 | ||
@@ -102,38 +88,8 @@ module XssTerminate | @@ -102,38 +88,8 @@ module XssTerminate | ||
102 | sanitizer = HTML5libSanitize.new | 88 | sanitizer = HTML5libSanitize.new |
103 | columns = sanitize_columns(:html5lib) | 89 | columns = sanitize_columns(:html5lib) |
104 | columns.each do |column| | 90 | columns.each do |column| |
105 | - sanitize_field(sanitizer, column.to_sym, columns_serialized.include?(column), :html5lib) | ||
106 | - end | ||
107 | - end | ||
108 | - | ||
109 | - def wellformed_html_code?(field) | ||
110 | - return true if !field | ||
111 | - counter = 0 | ||
112 | - in_comment = false | ||
113 | - field=field.split(//) | ||
114 | - for i in 0..field.length-1 | ||
115 | - if !in_comment | ||
116 | - if field[i] == '<' | ||
117 | - if field[i+1..i+3] == ["!","-","-"] | ||
118 | - in_comment = true | ||
119 | - else | ||
120 | - counter += 1 | ||
121 | - end | ||
122 | - elsif field[i] == '>' | ||
123 | - counter -= 1 | ||
124 | - end | ||
125 | - else | ||
126 | - if field[i-2..i] == ["-","-",">"] | ||
127 | - in_comment = false | ||
128 | - end | ||
129 | - end | ||
130 | - | ||
131 | - if counter < 0 || 1 < counter | ||
132 | - return false | ||
133 | - end | 91 | + sanitize_field(sanitizer, column.to_sym, columns_serialized.include?(column)) |
134 | end | 92 | end |
135 | - | ||
136 | - return counter == 0 | ||
137 | end | 93 | end |
138 | 94 | ||
139 | end | 95 | end |