From 6ea52ab5c0c29ce60f5a8ee2747eb52f47d5b884 Mon Sep 17 00:00:00 2001 From: John Dong Date: Mon, 20 Aug 2007 17:25:50 -0400 Subject: [PATCH] Add a bunch of fixes to HTML escaping, and a test case for it --- app/controllers/quickvote_controller.rb | 4 ++-- app/views/quickvote/_candidate_list.rhtml | 2 +- app/views/quickvote/_pref_table.rhtml | 6 ++--- app/views/quickvote/_result.rhtml | 5 ++-- app/views/quickvote/_victories_ties.rhtml | 4 ++-- app/views/quickvote/index.rhtml | 6 ++--- app/views/quickvote/results.rhtml | 24 ++++++++++---------- app/views/quickvote/thanks.rhtml | 2 +- app/views/site/index.rhtml | 2 +- test/functional/quickvote_controller_test.rb | 20 ++++++++++++++++ 10 files changed, 47 insertions(+), 28 deletions(-) diff --git a/app/controllers/quickvote_controller.rb b/app/controllers/quickvote_controller.rb index 5b3acab..bb60b45 100644 --- a/app/controllers/quickvote_controller.rb +++ b/app/controllers/quickvote_controller.rb @@ -14,7 +14,7 @@ class QuickvoteController < ApplicationController @quickvote = QuickVote.new(params[:quickvote]) # store the candidate grabbed through ajax and stored in flash @quickvote.candidatelist = flash[:candlist] - @quickvote.description=CGI.escapeHTML(@quickvote.description) + @quickvote.description=@quickvote.description # try to save, if it fails, show the page again (the flash should # still be intact if @quickvote.save @@ -33,7 +33,7 @@ class QuickvoteController < ApplicationController end def add_candidate - candidate_name = CGI.escapeHTML(params[:ajax][:newcandidate]) + candidate_name = params[:ajax][:newcandidate] unless candidate_name.strip.empty? if flash.has_key?(:candlist) and flash[:candlist].instance_of?(Array) flash[:candlist] << candidate_name unless flash[:candlist].index(candidate_name) diff --git a/app/views/quickvote/_candidate_list.rhtml b/app/views/quickvote/_candidate_list.rhtml index 64aa979..ddb47e5 100644 --- a/app/views/quickvote/_candidate_list.rhtml +++ b/app/views/quickvote/_candidate_list.rhtml @@ -3,7 +3,7 @@ <% if flash[:candlist] %> <% end %> diff --git a/app/views/quickvote/_pref_table.rhtml b/app/views/quickvote/_pref_table.rhtml index 4576a0f..1d3cdf3 100644 --- a/app/views/quickvote/_pref_table.rhtml +++ b/app/views/quickvote/_pref_table.rhtml @@ -9,17 +9,17 @@ <% candidates.each do |candidate| -%> - <%= names[candidate] -%> + <%=h names[candidate] -%> <% end -%> <% candidates.each do |winner| -%> - <%= names[winner] %> + <%=h names[winner] %> <% candidates.each do |loser| -%> <% if winner == loser -%> -- <% else %> <% wins = @election.condorcet_result.matrix[winner][loser]%> - <%= wins %> + <%=h wins %> <%= sparkline_tag [(wins.to_f/voters.to_f)*100.0], :type => 'pie', :diameter => 25, :share_color => '#74ce00' %> diff --git a/app/views/quickvote/_result.rhtml b/app/views/quickvote/_result.rhtml index e10890a..b643224 100644 --- a/app/views/quickvote/_result.rhtml +++ b/app/views/quickvote/_result.rhtml @@ -1,10 +1,9 @@ <% %> <% if result.winner? and result.winners.length == 1%>

The winner is: - <%= @candidates[result.winner].name.capitalize %>

+ <%=h @candidates[result.winner].name.capitalize %>

<% elsif result.winner? and result.winners.length > 1 %> -

There was a tie. The winners are: <%= - result.winners.collect {|w| @candidates[w].to_s.capitalize}.join(", ") %>

+

There was a tie. The winners are: <%=h( result.winners.collect {|w| @candidates[w].to_s.capitalize}.join(", ") )%>

<% else %>

There is no winner using this method.

<% end %> diff --git a/app/views/quickvote/_victories_ties.rhtml b/app/views/quickvote/_victories_ties.rhtml index 7c3506a..993caa8 100644 --- a/app/views/quickvote/_victories_ties.rhtml +++ b/app/views/quickvote/_victories_ties.rhtml @@ -4,9 +4,9 @@ <% victories.keys.each do |victor| %> - + <% victories[victor].keys.each do |loser| %> - + <% end -%> <% end -%> diff --git a/app/views/quickvote/index.rhtml b/app/views/quickvote/index.rhtml index e7cac3d..1179b7c 100644 --- a/app/views/quickvote/index.rhtml +++ b/app/views/quickvote/index.rhtml @@ -1,14 +1,14 @@ <% %> <% if @voter.election.shortdesc %> -

<%= @voter.election.shortdesc %>

+

<%=h @voter.election.shortdesc %>

<% else %>

QuickVote

<% end %> <% if @voter.election.longdesc %>

Description:

-
<%= @voter.election.longdesc %>
+
<%=h @voter.election.longdesc %>

Vote

<% end %> @@ -31,7 +31,7 @@ bottom. When you are done, press confirm to record your vote.

    <% for ranking in @voter.vote.rankings %>
  1. - <%= ranking.candidate.name.capitalize %>
    2. + <%=h ranking.candidate.name.capitalize %> <% end %>
diff --git a/app/views/quickvote/results.rhtml b/app/views/quickvote/results.rhtml index c49d682..799459d 100644 --- a/app/views/quickvote/results.rhtml +++ b/app/views/quickvote/results.rhtml @@ -4,7 +4,7 @@ <% if @election.shortdesc %>

Description:

-
<%= @election.shortdesc %> +
<%=h @election.shortdesc %> <% if @election.longdesc -%>
<%= h(@election.longdesc) -%> @@ -16,7 +16,7 @@
    <% for candidate in @election.candidates.sort %> -
  1. <%= candidate.name.capitalize %>
    2. +
  2. <%=h candidate.name.capitalize %>
    3. <% end %>
@@ -31,7 +31,7 @@

Schulze Method Results

<%= render :partial => 'result', :object => @election.ssd_result %> -

About the Schulze Method

The <%= link_to "Schulze method", @@ -50,7 +50,7 @@ Beatpath Winner, Path Voting, and Path Winner.

Plurality Results

<%= render :partial => 'result', :object => @election.plurality_result %> -

About Plurality Voting

<%= link_to "Plurality voting", @@ -70,7 +70,7 @@ voting.

(This algorithm assumes that top two choices are "approved.")

<%= render :partial => 'result', :object => @election.approval_result %> -

About Approval Voting

<%= link_to "Approval voting", @@ -88,7 +88,7 @@ accept or not.

Simple Condorcet Results

<%= render :partial => 'result', :object => @election.condorcet_result %> -

About Simple Cordorcet Voting

<%= link_to "Condorcet", @@ -108,7 +108,7 @@ another Condorcet system.

Borda Count Results

<%= render :partial => 'result', :object => @election.borda_result %> -

About Borda Count

<%= link_to "Borda count", @@ -125,7 +125,7 @@ points is the winner.

Instant Runoff (IRV) Results

-

About Instant Runoff Voting

<%= link_to "Instant runoff voting", @@ -159,16 +159,16 @@ by several other names.

<% raise ArgumentError.new, "Local Server" if voter.ipaddress == "127.0.0.1" %> <% raise ArgumentError.new, "XML-RPC Voter" if voter.ipaddress == "XMLRPC Request" %> <% w= Whois::Whois.new(IPAddr.new(voter.ipaddress).to_s,true)%> - <%=(w.host == nil or w.host.empty?) ? voter.ipaddress : w.host%> + <%=h((w.host == nil or w.host.empty?) ? voter.ipaddress : w.host)%>
- diff --git a/app/views/quickvote/thanks.rhtml b/app/views/quickvote/thanks.rhtml index 1975eb7..0fb3610 100644 --- a/app/views/quickvote/thanks.rhtml +++ b/app/views/quickvote/thanks.rhtml @@ -5,7 +5,7 @@ preferences:

    <% for rank in @voter.vote.rankings.sort %> -
  1. <%= rank.candidate.name.capitalize %>
    2. +
  2. <%=h rank.candidate.name.capitalize %>
    3. <% end %>
diff --git a/app/views/site/index.rhtml b/app/views/site/index.rhtml index 0d59849..1b994b0 100644 --- a/app/views/site/index.rhtml +++ b/app/views/site/index.rhtml @@ -17,7 +17,7 @@ methods.

diff --git a/test/functional/quickvote_controller_test.rb b/test/functional/quickvote_controller_test.rb index 076a104..02c133e 100644 --- a/test/functional/quickvote_controller_test.rb +++ b/test/functional/quickvote_controller_test.rb @@ -115,4 +115,24 @@ 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 + test_create_quickvote + qv=QuickVote.ident_to_quickvote('variable') + qv.description="foo" + qv.candidatelist = ["foo", "bar", ""] + qv.save! + get :index, { 'ident' => 'variable' } + assert_response :success + assert_no_tag :tag => "object" + assert_no_tag :tag => "foobar" + votes = QuickVote.ident_to_quickvote('variable').candidates.collect { |c| c.id} + post :confirm, { 'ident' => 'variable', 'rankings-list' => votes.sort_by {rand} } + assert_template('quickvote/thanks') + assert_no_tag :tag => "object" + assert_no_tag :tag => "foobar" + get :results, { 'ident' => 'variable' } + assert_response :success + assert_no_tag :tag => "object" + assert_no_tag :tag => "foobar" + end end -- 2.30.2
<%= names[victor] %><%=h names[victor] %><%= names[loser] %> (<%= victories[victor][loser] %>)<%=h names[loser] %> (<%= victories[victor][loser] %>)
<%w.search_whois%> - <%= (w.all.grep(/^(OrgName|org-name)/)[0] or "").sub(/^(OrgName|org-name)\:/,'').strip -%> - <%= (w.all.grep(/^(NetName|netname)/)[0] or "").sub(/^(NetName|netname)\:/,'').strip %> + <%=h (w.all.grep(/^(OrgName|org-name)/)[0] or "").sub(/^(OrgName|org-name)\:/,'').strip -%> - <%= (w.all.grep(/^(NetName|netname)/)[0] or "").sub(/^(NetName|netname)\:/,'').strip %> <% rescue ArgumentError => err %> - <%= err %> + <%=h err %> <%= err%> + <%=h err%> <% end %> <%= voter.vote.votestring %>