Add a bunch of fixes to HTML escaping, and a test case for it

diff --git a/app/controllers/quickvote_controller.rb b/app/controllers/quickvote_controller.rb
index 5b3acab79f21998d5897e90cc331b71745c37b8c..bb60b4552b029efff74be54bba9282c7599a671f 100644 (file)
--- 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 64aa97926c228cdb0813e0096965d4f005aec67d..ddb47e53fa7f6a49b707bcecdb530e5ef0dd1a54 100644 (file)
--- a/app/views/quickvote/_candidate_list.rhtml
+++ b/app/views/quickvote/_candidate_list.rhtml
@@ -3,7 +3,7 @@
 <% if flash[:candlist] %>
   <ul>
   <% for cand in flash[:candlist] %>
-    <li><%= cand.capitalize %></li>
+    <li><%=h cand.capitalize %></li>
   <% end %>
   </ul>
 <% end %>

diff --git a/app/views/quickvote/_pref_table.rhtml b/app/views/quickvote/_pref_table.rhtml
index 4576a0fafb8e7971af4499ba88b9501bacb1a2aa..1d3cdf37b29706c7be45c654ccf05000be20caef 100644 (file)
--- a/app/views/quickvote/_pref_table.rhtml
+++ b/app/views/quickvote/_pref_table.rhtml
@@ -9,17 +9,17 @@
   <tr>
        <td> </td>
        <% candidates.each do |candidate| -%>
-         <th><%= names[candidate] -%></th>
+         <th><%=h names[candidate] -%></th>
        <% end -%>
 <% candidates.each do |winner| -%>
   <tr>
-       <th><%= names[winner] %></th>
+       <th><%=h names[winner] %></th>
   <% candidates.each do |loser| -%> 
     <% if winner == loser -%>
       <td> -- </td>
     <% else %>         
       <td><% 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' %>
          </td>

diff --git a/app/views/quickvote/_result.rhtml b/app/views/quickvote/_result.rhtml
index e10890a7a92d80d5fbe11534eeb944d95e2b2828..b64322455fefcd8c554d5c1f362232c824db9df4 100644 (file)
--- a/app/views/quickvote/_result.rhtml
+++ b/app/views/quickvote/_result.rhtml
@@ -1,10 +1,9 @@
 <% %>
 <% if result.winner? and result.winners.length == 1%>
   <p><em>The winner is:
-     <strong><%= @candidates[result.winner].name.capitalize %></strong></em></p>
+     <strong><%=h @candidates[result.winner].name.capitalize %></strong></em></p>
 <% elsif result.winner? and result.winners.length > 1 %>
-  <p><em>There was a tie. The winners are: <strong><%=
-  result.winners.collect {|w| @candidates[w].to_s.capitalize}.join(", ") %></strong></em></p>
+  <p><em>There was a tie. The winners are: <strong><%=h( result.winners.collect {|w| @candidates[w].to_s.capitalize}.join(", ") )%></strong></em></p>
 <% else %>
   <p><em>There is no winner using this method. </em></strong></p>
 <% end %>

diff --git a/app/views/quickvote/_victories_ties.rhtml b/app/views/quickvote/_victories_ties.rhtml
index 7c3506a820f3d9037762b902e4f1a78f3692bbbd..993caa8f40280c65d460fc953283395198665a73 100644 (file)
--- a/app/views/quickvote/_victories_ties.rhtml
+++ b/app/views/quickvote/_victories_ties.rhtml
@@ -4,9 +4,9 @@
 <table class="voterbox">
   <% victories.keys.each do |victor| %>
   <tr>
-    <th><%= names[victor] %></th>
+    <th><%=h names[victor] %></th>
        <% victories[victor].keys.each do |loser| %>
-       <td><%= names[loser] %> (<%= victories[victor][loser] %>)</td>
+       <td><%=h names[loser] %> (<%= victories[victor][loser] %>)</td>
        <% end -%>
   </tr>
   <% end -%>

diff --git a/app/views/quickvote/index.rhtml b/app/views/quickvote/index.rhtml
index e7cac3d88f13de8d89892e233a4de5e192f7522e..1179b7cb894833f9f98d9f93da46958169d32428 100644 (file)
--- a/app/views/quickvote/index.rhtml
+++ b/app/views/quickvote/index.rhtml
@@ -1,14 +1,14 @@
 <% %>
 
 <% if @voter.election.shortdesc %>
-  <h1><%= @voter.election.shortdesc %></h1>
+  <h1><%=h @voter.election.shortdesc %></h1>
 <% else %>
   <h1>QuickVote</h1>
 <% end %>
 
 <% if @voter.election.longdesc %>
   <p><strong>Description:</strong></p>
-  <blockquote><%= @voter.election.longdesc %></blockquote>
+  <blockquote><%=h @voter.election.longdesc %></blockquote>
 
 <h2>Vote</h2>
 <% end %>
@@ -31,7 +31,7 @@ bottom</em>. When you are done, press confirm to record your vote.</p>
 <ol id="rankings-list">
   <% for ranking in @voter.vote.rankings %>
     <li class="moveable" id="ranking_<%= ranking.candidate.id %>">
-      <%= ranking.candidate.name.capitalize %></li>
+      <%=h ranking.candidate.name.capitalize %></li>
   <% end %>
 </ol>
 </div>

diff --git a/app/views/quickvote/results.rhtml b/app/views/quickvote/results.rhtml
index c49d68201a48e10363c14811cad171a6abe6dd39..799459df0f9704d5079271d4a37e34fa979a3152 100644 (file)
--- a/app/views/quickvote/results.rhtml
+++ b/app/views/quickvote/results.rhtml
@@ -4,7 +4,7 @@
 
 <% if @election.shortdesc %>
   <p><strong>Description:</strong></p>
-  <blockquote><em><%= @election.shortdesc %></em>
+  <blockquote><em><%=h @election.shortdesc %></em>
     <% if @election.longdesc -%>
       <br />
       <%= h(@election.longdesc) -%>
@@ -16,7 +16,7 @@
 
 <ol>
   <% for candidate in @election.candidates.sort %>
-    <li><%= candidate.name.capitalize %></li>
+    <li><%=h candidate.name.capitalize %></li>
   <% end %>
 </ol>
 
@@ -31,7 +31,7 @@
 <h3>Schulze Method Results</h3>
 <%= render :partial => 'result', :object => @election.ssd_result %>
 
-<div class="rbmoreinfo"
+<div class="rbmoreinfo">
 <h4>About the Schulze Method</h4>
 
 <p>The <%= link_to "Schulze method",
@@ -50,7 +50,7 @@ Beatpath Winner, Path Voting, and Path Winner.</p>
 <h3>Plurality Results</h3>
 <%= render :partial => 'result', :object => @election.plurality_result %>
 
-<div class="rbmoreinfo"
+<div class="rbmoreinfo">
 <h4>About Plurality Voting</h4>
 
 <p><%= link_to "Plurality voting",
@@ -70,7 +70,7 @@ voting.</p>
 <p><font size="-1">(This algorithm assumes that top two choices are "approved.")</font></p>
 <%= render :partial => 'result', :object => @election.approval_result %>
 
-<div class="rbmoreinfo"
+<div class="rbmoreinfo">
 <h4>About Approval Voting</h4>
 
 <p><%= link_to "Approval voting",
@@ -88,7 +88,7 @@ accept or not.</p>
 <h3>Simple Condorcet Results</h3>
 <%= render :partial => 'result', :object => @election.condorcet_result %>
 
-<div class="rbmoreinfo"
+<div class="rbmoreinfo">
 <h4>About Simple Cordorcet Voting</h4>
 
 <p><%= link_to "Condorcet",
@@ -108,7 +108,7 @@ another Condorcet system.</p>
 <h3>Borda Count Results</h3>
 <%= render :partial => 'result', :object => @election.borda_result %>
 
-<div class="rbmoreinfo"
+<div class="rbmoreinfo">
 <h4>About Borda Count</h4>
 
 <p><%= link_to "Borda count",
@@ -125,7 +125,7 @@ points is the winner.</p>
 <div class="resultbox">
 <h3>Instant Runoff (IRV) Results</h3>
 
-<div class="rbmoreinfo"
+<div class="rbmoreinfo">
 <h4>About Instant Runoff Voting</h4>
 
 <p><%= link_to "Instant runoff voting",
@@ -159,16 +159,16 @@ by several other names.</p>
         <% 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)%>
       </td>
       <td>
         <%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 %>
     </td>
-    <td><%= err%>
+    <td><%=h err%>
     <% end %>
     </td>
   <td><%= voter.vote.votestring %></td>

diff --git a/app/views/quickvote/thanks.rhtml b/app/views/quickvote/thanks.rhtml
index 1975eb705162f592c259219e0143c87e4f046bcb..0fb3610028e2bc2bc78a39424539b367bdfff673 100644 (file)
--- a/app/views/quickvote/thanks.rhtml
+++ b/app/views/quickvote/thanks.rhtml
@@ -5,7 +5,7 @@ preferences:</p>
 
 <ol>
   <% for rank in @voter.vote.rankings.sort %>
-    <li><%= rank.candidate.name.capitalize %> </li>
+    <li><%=h rank.candidate.name.capitalize %> </li>
   <% end %>
 </ol>
 

diff --git a/app/views/site/index.rhtml b/app/views/site/index.rhtml
index 0d59849a6997d2c621454e2491cca25284e3ed82..1b994b0e16ebf84e29e1b7df89c1c9298cb81e44 100644 (file)
--- a/app/views/site/index.rhtml
+++ b/app/views/site/index.rhtml
@@ -17,7 +17,7 @@ methods.</p>
 
 <ul>
 <% for quickvote in @quickvotes %>
-<li><%= link_to (quickvote.shortdesc || "Unnamed"), quickvote_url(:ident => quickvote.name) %></li>
+<li><%= link_to (h(quickvote.shortdesc) || "Unnamed"), quickvote_url(:ident => quickvote.name) %></li>
 <% end %>
 </ul>
 

diff --git a/test/functional/quickvote_controller_test.rb b/test/functional/quickvote_controller_test.rb
index 076a1047c0780dfe1c10a97f8b6a7475ae3ff6c1..02c133e1508c40c28d7fdd1443f1a916d9ccfa80 100644 (file)
--- 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="<object>foo</object>"
+    qv.candidatelist = ["<object>foo", "bar<object>", "<foobar>"]
+    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