From: Date: Mon, 11 Feb 2008 15:50:53 +0000 (-0500) Subject: merged changes back from live X-Git-Url: https://projects.mako.cc/source/selectricity-live/commitdiff_plain/215cf287e52da41605cb033da236a6b0aeee4f3a?hp=8a444bfe3f442135ba66e5d9b8cdddfa9e3b50f2 merged changes back from live --- diff --git a/TODO b/TODO index be982ee..f2e186e 100644 --- a/TODO +++ b/TODO @@ -1,2 +1,33 @@ Known bugs or issues: +- randomize each voting list + +- it'd be nice if the ordering were persistent, so I could drag + the candidates around, then come back later (loading the page anew), + make tweaks, and submit. + +- the results page says "the winner is Elizabeth Stark"; it'd be nice + if it were aware that it's a five-seat election and could say + explicitly who the whole board is. + +- it's probably a good thing that users can't check out what alternate + voting systems would have done at the click of a button. =) + (though of course they could work it out themselves.) + +- is there rhyme or reason to the ordering within each row of the + second table in pref_tables? it could be, eg, sorted by magnitude + of victory, narrowest first. + +- it'd be nice to attach names to the email identities, on eg the + details page; in this election we'd have said MIT for gregp@mit.edu, + Northeastern for cbudnick@gmail.com, etc. I don't know many of these + addresses, and names would be more informative. (They'd be more + helpful for auditing, too, as one could separately match names to the + correct list and match addresses to names.) + +- just a tiny suggestion - possibly it could be a good idea to add a + counter/line-count to the table with keys and vote results so in one + glance someone can see that the number of votes matches the number of + voters. + + 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 6b6d42f..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/election/detailed_results.rhtml b/app/views/election/detailed_results.rhtml index 4668570..eae756a 100644 --- a/app/views/election/detailed_results.rhtml +++ b/app/views/election/detailed_results.rhtml @@ -13,7 +13,7 @@

The voting rolls -- displayed here in alphabetical order -- for the last election are are follows.

-

Information is displayed here to help voters verify that their own +

Information is displayed here to help voters verifying that their own vote was recorded correctly and that the election was not tampered with.

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%>