From d38e768ed87787016eea78612ef79ac4cc119392 Mon Sep 17 00:00:00 2001
From: epriestley <git@epriestley.com>
Date: Mon, 5 Nov 2018 10:19:50 -0800
Subject: [PATCH] Prevent users from voting for invalid Slowvote options

Summary:
Depends on D19773. See <https://hackerone.com/reports/434116>. You can currently vote for invalid options by submitting, e.g., `vote[]=12345`.

By doing this, you can see the responses, which is sort of theoretically a security problem? This is definitely a bug, regardless.

Instead, only allow users to vote for options which are actually part of the poll.

Test Plan:
  - Tried to vote for invalid options by editing the form to `vote[]=12345` (got error).
  - Tried to vote for invalid options by editing the radio buttons on a plurality poll into checkboxes, checking multiple boxes, and submitting (got error).
  - Voted in approval and plurality polls the right way, from the main web UI and from the embed (`{V...}`) UI.

Reviewers: amckinley

Reviewed By: amckinley

Differential Revision: https://secure.phabricator.com/D19774
---
 .../PhabricatorSlowvoteVoteController.php     | 38 ++++++++++++-------
 1 file changed, 25 insertions(+), 13 deletions(-)

diff --git a/src/applications/slowvote/controller/PhabricatorSlowvoteVoteController.php b/src/applications/slowvote/controller/PhabricatorSlowvoteVoteController.php
index 53fbc0102d..62913b09e3 100644
--- a/src/applications/slowvote/controller/PhabricatorSlowvoteVoteController.php
+++ b/src/applications/slowvote/controller/PhabricatorSlowvoteVoteController.php
@@ -7,6 +7,10 @@ final class PhabricatorSlowvoteVoteController
     $viewer = $request->getViewer();
     $id = $request->getURIData('id');
 
+    if (!$request->isFormPost()) {
+      return id(new Aphront404Response());
+    }
+
     $poll = id(new PhabricatorSlowvoteQuery())
       ->setViewer($viewer)
       ->withIDs(array($id))
@@ -16,31 +20,36 @@ final class PhabricatorSlowvoteVoteController
     if (!$poll) {
       return new Aphront404Response();
     }
+
     if ($poll->getIsClosed()) {
       return new Aphront400Response();
     }
 
     $options = $poll->getOptions();
-    $viewer_choices = $poll->getViewerChoices($viewer);
+    $options = mpull($options, null, 'getID');
 
-    $old_votes = mpull($viewer_choices, null, 'getOptionID');
-
-    if (!$request->isFormPost()) {
-      return id(new Aphront404Response());
-    }
+    $old_votes = $poll->getViewerChoices($viewer);
+    $old_votes = mpull($old_votes, null, 'getOptionID');
 
     $votes = $request->getArr('vote');
     $votes = array_fuse($votes);
 
-    $this->updateVotes($viewer, $poll, $old_votes, $votes);
+    $method = $poll->getMethod();
+    $is_plurality = ($method == PhabricatorSlowvotePoll::METHOD_PLURALITY);
 
-    return id(new AphrontRedirectResponse())->setURI('/V'.$poll->getID());
-  }
+    if ($is_plurality && count($votes) > 1) {
+      throw new Exception(
+        pht('In this poll, you may only vote for one option.'));
+    }
 
-  private function updateVotes($viewer, $poll, $old_votes, $votes) {
-    if (!empty($votes) && count($votes) > 1 &&
-        $poll->getMethod() == PhabricatorSlowvotePoll::METHOD_PLURALITY) {
-      return id(new Aphront400Response());
+    foreach ($votes as $vote) {
+      if (!isset($options[$vote])) {
+        throw new Exception(
+          pht(
+            'Option ("%s") is not a valid poll option. You may only '.
+            'vote for valid options.',
+            $vote));
+      }
     }
 
     foreach ($old_votes as $old_vote) {
@@ -60,6 +69,9 @@ final class PhabricatorSlowvoteVoteController
         ->setOptionID($vote)
         ->save();
     }
+
+    return id(new AphrontRedirectResponse())
+      ->setURI($poll->getURI());
   }
 
 }