Skip to content

Commit d38e768

Browse files
author
epriestley
committed
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
1 parent 5e1d94f commit d38e768

File tree

1 file changed

+25
-13
lines changed

1 file changed

+25
-13
lines changed

src/applications/slowvote/controller/PhabricatorSlowvoteVoteController.php

Lines changed: 25 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,10 @@ public function handleRequest(AphrontRequest $request) {
77
$viewer = $request->getViewer();
88
$id = $request->getURIData('id');
99

10+
if (!$request->isFormPost()) {
11+
return id(new Aphront404Response());
12+
}
13+
1014
$poll = id(new PhabricatorSlowvoteQuery())
1115
->setViewer($viewer)
1216
->withIDs(array($id))
@@ -16,31 +20,36 @@ public function handleRequest(AphrontRequest $request) {
1620
if (!$poll) {
1721
return new Aphront404Response();
1822
}
23+
1924
if ($poll->getIsClosed()) {
2025
return new Aphront400Response();
2126
}
2227

2328
$options = $poll->getOptions();
24-
$viewer_choices = $poll->getViewerChoices($viewer);
25-
26-
$old_votes = mpull($viewer_choices, null, 'getOptionID');
29+
$options = mpull($options, null, 'getID');
2730

28-
if (!$request->isFormPost()) {
29-
return id(new Aphront404Response());
30-
}
31+
$old_votes = $poll->getViewerChoices($viewer);
32+
$old_votes = mpull($old_votes, null, 'getOptionID');
3133

3234
$votes = $request->getArr('vote');
3335
$votes = array_fuse($votes);
3436

35-
$this->updateVotes($viewer, $poll, $old_votes, $votes);
37+
$method = $poll->getMethod();
38+
$is_plurality = ($method == PhabricatorSlowvotePoll::METHOD_PLURALITY);
3639

37-
return id(new AphrontRedirectResponse())->setURI('/V'.$poll->getID());
38-
}
40+
if ($is_plurality && count($votes) > 1) {
41+
throw new Exception(
42+
pht('In this poll, you may only vote for one option.'));
43+
}
3944

40-
private function updateVotes($viewer, $poll, $old_votes, $votes) {
41-
if (!empty($votes) && count($votes) > 1 &&
42-
$poll->getMethod() == PhabricatorSlowvotePoll::METHOD_PLURALITY) {
43-
return id(new Aphront400Response());
45+
foreach ($votes as $vote) {
46+
if (!isset($options[$vote])) {
47+
throw new Exception(
48+
pht(
49+
'Option ("%s") is not a valid poll option. You may only '.
50+
'vote for valid options.',
51+
$vote));
52+
}
4453
}
4554

4655
foreach ($old_votes as $old_vote) {
@@ -60,6 +69,9 @@ private function updateVotes($viewer, $poll, $old_votes, $votes) {
6069
->setOptionID($vote)
6170
->save();
6271
}
72+
73+
return id(new AphrontRedirectResponse())
74+
->setURI($poll->getURI());
6375
}
6476

6577
}

0 commit comments

Comments
 (0)