From a356aac56f17278e79372b4e63ff3e160eec7cd2 Mon Sep 17 00:00:00 2001 From: Date: Mon, 11 Feb 2008 10:49:30 -0500 Subject: [PATCH 1/1] added the ability to add safe html tags to input (i.e., images) - added new white_list_plugin - changed it so that it is being used for candidate names in quickvotes --- app/helpers/application_helper.rb | 1 - app/views/common/_methodinfo_ssd.rhtml | 2 +- app/views/common/_pref_tables.rhtml | 8 +- app/views/common/_result.rhtml | 4 +- app/views/common/_sortable_vote.rhtml | 2 +- app/views/quickvote/_approval_table.rhtml | 4 +- app/views/quickvote/_candidate_list.rhtml | 2 +- app/views/quickvote/results.rhtml | 2 +- app/views/quickvote/thanks.rhtml | 2 +- app/views/voter/details.rhtml | 10 +- test/functional/quickvote_controller_test.rb | 18 ++- vendor/plugins/white_list/README | 29 ++++ vendor/plugins/white_list/Rakefile | 22 +++ vendor/plugins/white_list/init.rb | 2 + .../white_list/lib/white_list_helper.rb | 97 +++++++++++++ .../white_list/test/white_list_test.rb | 132 ++++++++++++++++++ 16 files changed, 318 insertions(+), 19 deletions(-) create mode 100644 vendor/plugins/white_list/README create mode 100644 vendor/plugins/white_list/Rakefile create mode 100644 vendor/plugins/white_list/init.rb create mode 100644 vendor/plugins/white_list/lib/white_list_helper.rb create mode 100644 vendor/plugins/white_list/test/white_list_test.rb diff --git a/app/helpers/application_helper.rb b/app/helpers/application_helper.rb index 4752f00..22a7940 100644 --- a/app/helpers/application_helper.rb +++ b/app/helpers/application_helper.rb @@ -1,4 +1,3 @@ # Methods added to this helper will be available to all templates in the application. module ApplicationHelper - end diff --git a/app/views/common/_methodinfo_ssd.rhtml b/app/views/common/_methodinfo_ssd.rhtml index 0c8dc95..5ec8b89 100644 --- a/app/views/common/_methodinfo_ssd.rhtml +++ b/app/views/common/_methodinfo_ssd.rhtml @@ -3,7 +3,7 @@ preference (from most preferred to least preferred):

    <% @election.ssd_result.ranked_candidates.each do |place| %> -
  1. <%= h(place.collect {|c| @names[c].capitalize}.join( " and " )) %> +
  2. <%= white_list place.collect {|c| @names[c].capitalize}.join( " and " ) %> <%= "(TIE)" if place.length > 1 %>
    3. <% end %>
diff --git a/app/views/common/_pref_tables.rhtml b/app/views/common/_pref_tables.rhtml index 7701985..567873b 100644 --- a/app/views/common/_pref_tables.rhtml +++ b/app/views/common/_pref_tables.rhtml @@ -14,13 +14,13 @@ top of the left column.

<% candidates.each do |candidate| -%> - <%=h @names[candidate] -%> + <%= white_list(@names[candidate]) -%> <% end -%> <% candidates.each do |winner| -%> - <%=h @names[winner] %> + <%= white_list(@names[winner]) %> <% candidates.each do |loser| -%> <% if winner == loser -%> -- @@ -46,10 +46,10 @@ parenthesis.

<% candidates.each do |victor| %> - + <% victories[victor].keys.each do |loser| %> <% margin = victories[victor][loser]%> - <% @election.approval_result.points.keys.sort.each do |candidate| %> - + <% end -%> @@ -12,4 +12,4 @@ <% end -%> -
<%=h @names[victor] %><%= white_list(@names[victor]) %><%=h @names[loser] %> + <%= white_list(@names[loser]) %> <% if margin == 0%> Tied! <% else -%> diff --git a/app/views/common/_result.rhtml b/app/views/common/_result.rhtml index b0a3a85..ea0ec40 100644 --- a/app/views/common/_result.rhtml +++ b/app/views/common/_result.rhtml @@ -2,9 +2,9 @@

<% if result.winner? and result.winners.length == 1 -%> The winner is: - <%=h @candidates[result.winner].name.capitalize %> + <%= white_list(@candidates[result.winner].name.capitalize) %> <% elsif result.winner? and result.winners.length > 1 %> - There was a tie. The winners are: <%=h( result.winners.collect {|w| @candidates[w].to_s.capitalize}.join(", ") )%> + There was a tie. The winners are: <%= white_list(result.winners.collect {|w| @candidates[w].to_s.capitalize}.join(", ") )%> <% else %>

There is no winner using this method. <% end %> diff --git a/app/views/common/_sortable_vote.rhtml b/app/views/common/_sortable_vote.rhtml index 6876966..f026c42 100644 --- a/app/views/common/_sortable_vote.rhtml +++ b/app/views/common/_sortable_vote.rhtml @@ -2,7 +2,7 @@

    <% for ranking in @voter.vote.rankings %>
  1. - <%=h ranking.candidate.name.capitalize %>
    2. + <%= white_list(ranking.candidate.name.capitalize) %> <% end %>
diff --git a/app/views/quickvote/_approval_table.rhtml b/app/views/quickvote/_approval_table.rhtml index 85185ee..1c16d1d 100644 --- a/app/views/quickvote/_approval_table.rhtml +++ b/app/views/quickvote/_approval_table.rhtml @@ -2,7 +2,7 @@
Candidate<%=h @names[candidate] %><%= white_list(@names[candidate]) %>
<%= points %>
\ No newline at end of file + diff --git a/app/views/quickvote/_candidate_list.rhtml b/app/views/quickvote/_candidate_list.rhtml index 4ec3db8..9ccb3b1 100644 --- a/app/views/quickvote/_candidate_list.rhtml +++ b/app/views/quickvote/_candidate_list.rhtml @@ -2,7 +2,7 @@ <% if flash[:candidate_names] %> <% end %> diff --git a/app/views/quickvote/results.rhtml b/app/views/quickvote/results.rhtml index 8d8bf5f..78e74e8 100644 --- a/app/views/quickvote/results.rhtml +++ b/app/views/quickvote/results.rhtml @@ -23,7 +23,7 @@
    <% for candidate in @election.candidates.sort %> -
  1. <%=h candidate.name.capitalize %>
    2. +
  2. <%= white_list(candidate.name.capitalize) %>
    3. <% end %>
diff --git a/app/views/quickvote/thanks.rhtml b/app/views/quickvote/thanks.rhtml index bf98ef8..d6ed8e0 100644 --- a/app/views/quickvote/thanks.rhtml +++ b/app/views/quickvote/thanks.rhtml @@ -8,7 +8,7 @@ preferences:

    <% for rank in @voter.vote.rankings.sort %> -
  1. <%=h rank.candidate.name.capitalize %>
    2. +
  2. <%= white_list(rank.candidate.name.capitalize) %>
    3. <% end %>
diff --git a/app/views/voter/details.rhtml b/app/views/voter/details.rhtml index 584c408..cf73f41 100644 --- a/app/views/voter/details.rhtml +++ b/app/views/voter/details.rhtml @@ -4,7 +4,7 @@

This page contains information useful for auditing elections and -verify that votes were tabulated correctly.

+verifying that votes were tabulated correctly.

The following invididuals (in random order) voted in this election:

@@ -20,15 +20,17 @@ election:

The column marked Verification Token lists tokens that were given to voters at the time of voting. Voters can check to see that the vote that corresponds to their token was recorded correctly. The column -marks "vote" lists the candidates in order of the voter's preference. To -read these votes, please refer to the key below.

+marked Vote lists the candidates in order of the voter's +preference. To read these votes, refer to the key below.

+ -<%- @votes.each do |vote| -%> +<%- @votes.each_with_index do |vote, i| -%> + <%- end -%> diff --git a/test/functional/quickvote_controller_test.rb b/test/functional/quickvote_controller_test.rb index 60ddb1b..e116c0d 100644 --- a/test/functional/quickvote_controller_test.rb +++ b/test/functional/quickvote_controller_test.rb @@ -115,24 +115,40 @@ class QuickvoteControllerTest < Test::Unit::TestCase post :confirm, { 'ident' => 'variable', 'rankings-list' => votes.sort_by {rand} } assert_redirected_to :controller => 'quickvote', :ident => 'variable' end + def test_display_tainted_quickvote + # create quickvote with tainted data test_create_quickvote qv=QuickVote.ident_to_quickvote('variable') qv.description="foo" - qv.candidate_names = ["foo", "bar", ""] + qv.candidate_names = ["foo", "bar", "", + 'bar'] qv.save! + + # display the vote/index page and check for bad tags and the ability + # to make an image tag get :index, { 'ident' => 'variable' } assert_response :success assert_no_tag :tag => "object" assert_no_tag :tag => "foobar" + assert_tag :tag => "img", + :parent => { :tag => "li", :attributes => { :class => "moveable" } } + + # actually vote votes = QuickVote.ident_to_quickvote('variable').candidates.collect { |c| c.id} post :confirm, { 'ident' => 'variable', 'rankings-list' => votes.sort_by {rand} } + + # check for bad/good tags assert_template('quickvote/thanks') assert_no_tag :tag => "object" assert_no_tag :tag => "foobar" + assert_tag :tag => "img", :parent => { :tag => "li" } + + # get the results page and check for good/bad tags get :results, { 'ident' => 'variable' } assert_response :success assert_no_tag :tag => "object" assert_no_tag :tag => "foobar" + assert_tag :tag => "img", :parent => { :tag => "li" } end end diff --git a/vendor/plugins/white_list/README b/vendor/plugins/white_list/README new file mode 100644 index 0000000..84bb1ee --- /dev/null +++ b/vendor/plugins/white_list/README @@ -0,0 +1,29 @@ +WhiteList +========= + +This White Listing helper will html encode all tags and strip all attributes that aren't specifically allowed. +It also strips href/src tags with invalid protocols, like javascript: especially. It does its best to counter any +tricks that hackers may use, like throwing in unicode/ascii/hex values to get past the javascript: filters. Check out +the extensive test suite. + + <%= white_list @article.body %> + +You can add or remove tags/attributes if you want to customize it a bit. + +Add table tags + + WhiteListHelper.tags.merge %w(table td th) + +Remove tags + + WhiteListHelper.tags.delete 'div' + +Change allowed attributes + + WhiteListHelper.attributes.merge %w(id class style) + +white_list accepts a block for custom tag escaping. Shown below is the default block that white_list uses if none is given. +The block is called for all bad tags, and every text node. node is an instance of HTML::Node (either HTML::Tag or HTML::Text). +bad is nil for text nodes inside good tags, or is the tag name of the bad tag. + + <%= white_list(@article.body) { |node, bad| white_listed_bad_tags.include?(bad) ? nil : node.to_s.gsub(/ \ No newline at end of file diff --git a/vendor/plugins/white_list/Rakefile b/vendor/plugins/white_list/Rakefile new file mode 100644 index 0000000..ce067be --- /dev/null +++ b/vendor/plugins/white_list/Rakefile @@ -0,0 +1,22 @@ +require 'rake' +require 'rake/testtask' +require 'rake/rdoctask' + +desc 'Default: run unit tests.' +task :default => :test + +desc 'Test the white_list plugin.' +Rake::TestTask.new(:test) do |t| + t.libs << 'lib' + t.pattern = 'test/**/*_test.rb' + t.verbose = true +end + +desc 'Generate documentation for the white_list plugin.' +Rake::RDocTask.new(:rdoc) do |rdoc| + rdoc.rdoc_dir = 'rdoc' + rdoc.title = 'WhiteList' + rdoc.options << '--line-numbers' << '--inline-source' + rdoc.rdoc_files.include('README') + rdoc.rdoc_files.include('lib/**/*.rb') +end diff --git a/vendor/plugins/white_list/init.rb b/vendor/plugins/white_list/init.rb new file mode 100644 index 0000000..92bf8ea --- /dev/null +++ b/vendor/plugins/white_list/init.rb @@ -0,0 +1,2 @@ +require 'white_list_helper' +ActionView::Base.send :include, WhiteListHelper \ No newline at end of file diff --git a/vendor/plugins/white_list/lib/white_list_helper.rb b/vendor/plugins/white_list/lib/white_list_helper.rb new file mode 100644 index 0000000..52bbb91 --- /dev/null +++ b/vendor/plugins/white_list/lib/white_list_helper.rb @@ -0,0 +1,97 @@ +module WhiteListHelper + @@protocol_attributes = Set.new %w(src href) + @@protocol_separator = /:|(�*58)|(p)|(%|%)3A/ + mattr_reader :protocol_attributes, :protocol_separator + + def self.contains_bad_protocols?(white_listed_protocols, value) + value =~ protocol_separator && !white_listed_protocols.include?(value.split(protocol_separator).first) + end + + klass = class << self; self; end + klass_methods = [] + inst_methods = [] + [:bad_tags, :tags, :attributes, :protocols].each do |attr| + # Add class methods to the module itself + klass_methods << <<-EOS + def #{attr}=(value) @@#{attr} = Set.new(value) end + def #{attr}() @@#{attr} end + EOS + + # prefix the instance methods with white_listed_* + inst_methods << "def white_listed_#{attr}() ::WhiteListHelper.#{attr} end" + end + + klass.class_eval klass_methods.join("\n"), __FILE__, __LINE__ + class_eval inst_methods.join("\n"), __FILE__, __LINE__ + + # This White Listing helper will html encode all tags and strip all attributes that aren't specifically allowed. + # It also strips href/src tags with invalid protocols, like javascript: especially. It does its best to counter any + # tricks that hackers may use, like throwing in unicode/ascii/hex values to get past the javascript: filters. Check out + # the extensive test suite. + # + # <%= white_list @article.body %> + # + # You can add or remove tags/attributes if you want to customize it a bit. + # + # Add table tags + # + # WhiteListHelper.tags.merge %w(table td th) + # + # Remove tags + # + # WhiteListHelper.tags.delete 'div' + # + # Change allowed attributes + # + # WhiteListHelper.attributes.merge %w(id class style) + # + # white_list accepts a block for custom tag escaping. Shown below is the default block that white_list uses if none is given. + # The block is called for all bad tags, and every text node. node is an instance of HTML::Node (either HTML::Tag or HTML::Text). + # bad is nil for text nodes inside good tags, or is the tag name of the bad tag. + # + # <%= white_list(@article.body) { |node, bad| white_listed_bad_tags.include?(bad) ? nil : node.to_s.gsub(/ + # + def white_list(html, options = {}, &block) + return html if html.blank? || !html.include?('<') + attrs = Set.new(options[:attributes]).merge(white_listed_attributes) + tags = Set.new(options[:tags] ).merge(white_listed_tags) + block ||= lambda { |node, bad| white_listed_bad_tags.include?(bad) ? nil : node.to_s.gsub(/foo bar baz end", %(start <#{tag_name} title="1">foo <bad>bar</bad> baz end) + end + end + + def test_should_allow_anchors + assert_white_listed %(), %() + end + + %w(src width height alt).each do |img_attr| + define_method "test_should_allow_image_#{img_attr}_attribute" do + assert_white_listed %(), %() + end + end + + def test_should_handle_non_html + assert_white_listed 'abc' + end + + def test_should_handle_blank_text + assert_white_listed nil + assert_white_listed '' + end + + def test_should_allow_custom_tags + text = "foo" + assert_equal(text, white_list(text, :tags => %w(u))) + end + + def test_should_allow_custom_tags_with_attributes + text = %(
foo
) + assert_equal(text, white_list(text, :attributes => ['foo'])) + end + + [%w(img src), %w(a href)].each do |(tag, attr)| + define_method "test_should_strip_#{attr}_attribute_in_#{tag}_with_bad_protocols" do + assert_white_listed %(<#{tag} #{attr}="javascript:bang" title="1">boo), %(<#{tag} title="1">boo) + end + end + + def test_should_flag_bad_protocols + %w(about chrome data disk hcp help javascript livescript lynxcgi lynxexec ms-help ms-its mhtml mocha opera res resource shell vbscript view-source vnd.ms.radio wysiwyg).each do |proto| + assert contains_bad_protocols?("#{proto}://bad") + end + end + + def test_should_accept_good_protocols + WhiteListHelper.protocols.each do |proto| + assert !contains_bad_protocols?("#{proto}://good") + end + end + + def test_should_reject_hex_codes_in_protocol + assert contains_bad_protocols?("%6A%61%76%61%73%63%72%69%70%74%3A%61%6C%65%72%74%28%22%58%53%53%22%29") + assert_white_listed %(1), "1" + end + + def test_should_block_script_tag + assert_white_listed %(), "" + end + + [%(), + %(), + %(), + %(">), + %(), + %(), + %(), + %(), + %(), + %(), + %(), + %(), + %(), + %(), + %()].each_with_index do |img_hack, i| + define_method "test_should_not_fall_for_xss_image_hack_#{i}" do + assert_white_listed img_hack, "" + end + end + + def test_should_sanitize_tag_broken_up_by_null + assert_white_listed %(alert(\"XSS\")), "<scr>alert(\"XSS\")</scr>" + end + + def test_should_sanitize_invalid_script_tag + assert_white_listed %(), "" + end + + def test_should_sanitize_script_tag_with_multiple_open_brackets + assert_white_listed %(<), "<" + assert_white_listed %(
Verification Token Vote
<%= i + 1 %> <%= vote.token %><%= vote.votestring%>