Commit 78e55056e17b388ffe594928c0152609be9fc512
Committed by
Joenio Costa
1 parent
e8cad054
Exists in
master
and in
29 other branches
Xss_terminate must not escape html comments
(ActionItem1540)
Showing
6 changed files
with
68 additions
and
8 deletions
Show diff stats
test/unit/environment_test.rb
... | ... | @@ -879,4 +879,12 @@ class EnvironmentTest < Test::Unit::TestCase |
879 | 879 | assert_no_match /[<>]/, environment.message_for_disabled_enterprise |
880 | 880 | end |
881 | 881 | |
882 | + should 'not sanitize html comments' do | |
883 | + environment = Environment.new | |
884 | + environment.message_for_disabled_enterprise = '<p><!-- <asdf> << aasdfa >>> --> <h1> Wellformed html code </h1>' | |
885 | + environment.valid? | |
886 | + | |
887 | + assert_match /<!-- .* --> <h1> Wellformed html code <\/h1>/, environment.message_for_disabled_enterprise | |
888 | + end | |
889 | + | |
882 | 890 | end | ... | ... |
test/unit/event_test.rb
... | ... | @@ -250,4 +250,14 @@ class EventTest < ActiveSupport::TestCase |
250 | 250 | assert_no_match /[<>]/, event.address |
251 | 251 | end |
252 | 252 | |
253 | + should 'not sanitize html comments' do | |
254 | + event = Event.new | |
255 | + event.description = '<p><!-- <asdf> << aasdfa >>> --> <h1> Wellformed html code </h1>' | |
256 | + event.address = '<p><!-- <asdf> << aasdfa >>> --> <h1> Wellformed html code </h1>' | |
257 | + event.valid? | |
258 | + | |
259 | + assert_match /<!-- .* --> <h1> Wellformed html code <\/h1>/, event.description | |
260 | + assert_match /<!-- .* --> <h1> Wellformed html code <\/h1>/, event.address | |
261 | + end | |
262 | + | |
253 | 263 | end | ... | ... |
test/unit/folder_test.rb
... | ... | @@ -140,6 +140,14 @@ class FolderTest < ActiveSupport::TestCase |
140 | 140 | assert_equal "<h1> Body </h1>", folder.body |
141 | 141 | end |
142 | 142 | |
143 | + should 'not sanitize html comments' do | |
144 | + folder = Folder.new | |
145 | + folder.body = '<p><!-- <asdf> << aasdfa >>> --> <h1> Wellformed html code </h1>' | |
146 | + folder.valid? | |
147 | + | |
148 | + assert_match /<!-- .* --> <h1> Wellformed html code <\/h1>/, folder.body | |
149 | + end | |
150 | + | |
143 | 151 | should 'escape malformed html tags' do |
144 | 152 | folder = Folder.new |
145 | 153 | folder.body = "<h1<< Description >>/h1>" | ... | ... |
test/unit/profile_test.rb
... | ... | @@ -1553,6 +1553,16 @@ class ProfileTest < Test::Unit::TestCase |
1553 | 1553 | assert_no_match /[<>]/, profile.custom_footer |
1554 | 1554 | end |
1555 | 1555 | |
1556 | + should 'not sanitize html comments' do | |
1557 | + profile = Profile.new | |
1558 | + profile.custom_header = '<p><!-- <asdf> << aasdfa >>> --> <h1> Wellformed html code </h1>' | |
1559 | + profile.custom_footer = '<p><!-- <asdf> << aasdfa >>> --> <h1> Wellformed html code </h1>' | |
1560 | + profile.valid? | |
1561 | + | |
1562 | + assert_match /<!-- .* --> <h1> Wellformed html code <\/h1>/, profile.custom_header | |
1563 | + assert_match /<!-- .* --> <h1> Wellformed html code <\/h1>/, profile.custom_footer | |
1564 | + end | |
1565 | + | |
1556 | 1566 | private |
1557 | 1567 | |
1558 | 1568 | def assert_invalid_identifier(id) | ... | ... |
test/unit/tiny_mce_article_test.rb
... | ... | @@ -74,4 +74,13 @@ class TinyMceArticleTest < Test::Unit::TestCase |
74 | 74 | article = TinyMceArticle.create!(:profile => profile, :name => 'article', :abstract => 'abstract', :body => "<embed flashvars='config={"key":"\#$b6eb72a0f2f1e29f3d4"}'> </embed>") |
75 | 75 | assert_equal "<embed flashvars=\"config={"key":"\#$b6eb72a0f2f1e29f3d4"}\"> </embed>", article.body |
76 | 76 | end |
77 | + | |
78 | + should 'not sanitize html comments' do | |
79 | + article = TinyMceArticle.new | |
80 | + article.body = '<p><!-- <asdf> << aasdfa >>> --> <h1> Wellformed html code </h1>' | |
81 | + article.valid? | |
82 | + | |
83 | + assert_match /<!-- .* --> <h1> Wellformed html code <\/h1>/, article.body | |
84 | + end | |
85 | + | |
77 | 86 | end | ... | ... |
vendor/plugins/xss_terminate/lib/xss_terminate.rb
... | ... | @@ -53,7 +53,7 @@ module XssTerminate |
53 | 53 | if with == :full |
54 | 54 | self[field] = CGI.escapeHTML(self[field]) |
55 | 55 | elsif with == :white_list |
56 | - self[field] = CGI.escapeHTML(self[field]) if !wellformed_html_tag?(self[field]) | |
56 | + self[field] = CGI.escapeHTML(self[field]) if !wellformed_html_code?(self[field]) | |
57 | 57 | end |
58 | 58 | |
59 | 59 | else |
... | ... | @@ -62,7 +62,7 @@ module XssTerminate |
62 | 62 | if with == :full |
63 | 63 | self.send("#{field}=", CGI.escapeHTML(self.send("#{field}"))) |
64 | 64 | elsif with == :white_list |
65 | - self.send("#{field}=", CGI.escapeHTML(self.send("#{field}"))) if !wellformed_html_tag?(self.send("#{field}")) | |
65 | + self.send("#{field}=", CGI.escapeHTML(self.send("#{field}"))) if !wellformed_html_code?(self.send("#{field}")) | |
66 | 66 | end |
67 | 67 | |
68 | 68 | end |
... | ... | @@ -103,14 +103,29 @@ module XssTerminate |
103 | 103 | end |
104 | 104 | end |
105 | 105 | |
106 | - def wellformed_html_tag?(field) | |
106 | + def wellformed_html_code?(field) | |
107 | 107 | return true if !field |
108 | - | |
109 | 108 | counter = 0 |
110 | - field.split(//).each do |letter| | |
111 | - counter += 1 if letter == '<' | |
112 | - counter -= 1 if letter == '>' | |
113 | - if counter < 0 || 1 < counter | |
109 | + in_comment = false | |
110 | + field=field.split(//) | |
111 | + for i in 0..field.length-1 | |
112 | + if !in_comment | |
113 | + if field[i] == '<' | |
114 | + if field[i+1..i+3] == ["!","-","-"] | |
115 | + in_comment = true | |
116 | + else | |
117 | + counter += 1 | |
118 | + end | |
119 | + elsif field[i] == '>' | |
120 | + counter -= 1 | |
121 | + end | |
122 | + else | |
123 | + if field[i-2..i] == ["-","-",">"] | |
124 | + in_comment = false | |
125 | + end | |
126 | + end | |
127 | + | |
128 | + if counter < 0 || 1 < counter | |
114 | 129 | return false |
115 | 130 | end |
116 | 131 | end | ... | ... |