Commit 30af88087b172d9fb46bc7ffbabe26be0af7cdb0
1 parent
ba92876a
Exists in
master
and in
22 other branches
ActionItem435: not escaping HTML
but filtering it to avoid XSS attacks git-svn-id: https://svn.colivre.coop.br/svn/noosfero/trunk@2023 3f533792-8f58-4932-b0fe-aaf55b0a4547
Showing
2 changed files
with
24 additions
and
2 deletions
Show diff stats
app/models/event.rb
| @@ -6,6 +6,8 @@ class Event < Article | @@ -6,6 +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 ], :with => 'white_list' | ||
| 10 | + | ||
| 9 | validates_presence_of :title, :start_date | 11 | validates_presence_of :title, :start_date |
| 10 | 12 | ||
| 11 | validates_each :start_date do |event,field,value| | 13 | validates_each :start_date do |event,field,value| |
| @@ -77,10 +79,10 @@ class Event < Article | @@ -77,10 +79,10 @@ class Event < Article | ||
| 77 | } | 79 | } |
| 78 | } | 80 | } |
| 79 | 81 | ||
| 80 | - html.div self.description | 82 | + html.div '_____XXXX_DESCRIPTION_GOES_HERE_XXXX_____' |
| 81 | } | 83 | } |
| 82 | 84 | ||
| 83 | - result | 85 | + result.sub('_____XXXX_DESCRIPTION_GOES_HERE_XXXX_____', self.description) |
| 84 | end | 86 | end |
| 85 | 87 | ||
| 86 | def link=(value) | 88 | def link=(value) |
test/unit/event_test.rb
| @@ -149,6 +149,20 @@ class EventTest < ActiveSupport::TestCase | @@ -149,6 +149,20 @@ class EventTest < ActiveSupport::TestCase | ||
| 149 | assert_equal 'http://www.gnu.org', a.link | 149 | assert_equal 'http://www.gnu.org', a.link |
| 150 | end | 150 | end |
| 151 | 151 | ||
| 152 | + should 'not escape HTML in description' do | ||
| 153 | + a = Event.new(:description => '<p>a paragraph of text</p>', :link => 'www.gnu.org') | ||
| 154 | + | ||
| 155 | + assert_match '<p>a paragraph of text</p>', a.to_html | ||
| 156 | + end | ||
| 157 | + | ||
| 158 | + should 'filter HTML in description' do | ||
| 159 | + profile = create_user('testuser').person | ||
| 160 | + e = Event.create!(:profile => profile, :name => 'test', :description => '<p>a paragraph (valid)</p><script type="text/javascript">/* this is invalid */</script>"', :link => 'www.colivre.coop.br', :start_date => Date.today) | ||
| 161 | + | ||
| 162 | + assert_tag_in_string e.description, :tag => 'p', :content => 'a paragraph (valid)' | ||
| 163 | + assert_no_tag_in_string e.description, :tag => 'script' | ||
| 164 | + end | ||
| 165 | + | ||
| 152 | protected | 166 | protected |
| 153 | 167 | ||
| 154 | def assert_tag_in_string(text, options) | 168 | def assert_tag_in_string(text, options) |
| @@ -157,4 +171,10 @@ class EventTest < ActiveSupport::TestCase | @@ -157,4 +171,10 @@ class EventTest < ActiveSupport::TestCase | ||
| 157 | assert tag, "expected tag #{options.inspect}, but not found in #{text.inspect}" | 171 | assert tag, "expected tag #{options.inspect}, but not found in #{text.inspect}" |
| 158 | end | 172 | end |
| 159 | 173 | ||
| 174 | + def assert_no_tag_in_string(text, options) | ||
| 175 | + doc = HTML::Document.new(text, false, false) | ||
| 176 | + tag = doc.find(options) | ||
| 177 | + assert !tag, "expected no tag #{options.inspect}, but tag found in #{text.inspect}" | ||
| 178 | + end | ||
| 179 | + | ||
| 160 | end | 180 | end |