From 4fc330ea9adf57d161e4b7238127165d01525dfe Mon Sep 17 00:00:00 2001 From: "justdave%syndicomm.com" <> Date: Wed, 6 Jun 2001 11:51:55 +0000 Subject: Fix for bug 39557: doeditvotes.cgi will no longer create a vote record for a nonexistant bug if the HTML is tampered with or other bugs cause bad bug numbers in the submitted form. Patch by Myk Melez r= justdave@syndicomm.com --- doeditvotes.cgi | 116 ++++++++++++++++++++++++++++++++------------------------ 1 file changed, 66 insertions(+), 50 deletions(-) (limited to 'doeditvotes.cgi') diff --git a/doeditvotes.cgi b/doeditvotes.cgi index 7b830595a..155635723 100755 --- a/doeditvotes.cgi +++ b/doeditvotes.cgi @@ -25,11 +25,37 @@ use strict; require "CGI.pl"; +ConnectToDatabase(); + confirm_login(); +###################################################################### +# Begin Data/Security Validation +###################################################################### + +# Build a list of bug IDs for which votes have been submitted. Votes +# are submitted in form fields in which the field names are the bug +# IDs and the field values are the number of votes. +my @buglist = grep {/^[1-9][0-9]*$/} keys(%::FORM); + +# Call ValidateBugID on each bug ID to make sure it is a positive +# integer representing an existing bug that the user is authorized +# to access, and make sure the number of votes submitted is also +# a non-negative integer (a series of digits not preceded by a +# minus sign). +foreach my $id (@buglist) { + ValidateBugID($id); + ($::FORM{$id} =~ /^\d+$/) + || DisplayError("Only use non-negative numbers for your bug votes.") + && exit; +} + +###################################################################### +# End Data/Security Validation +###################################################################### + print "Content-type: text/html\n\n"; -ConnectToDatabase(); GetVersionTable(); my $who = DBNameToIdAndCheck($::COOKIE{'Bugzilla_login'}); @@ -43,60 +69,50 @@ if ( (! defined $who) || (!$who) ) { exit(); } -my @buglist = grep {/^\d+$/} keys(%::FORM); - -if (0 == @buglist) { - PutHeader("Oops?"); - print "Something got confused. Please click Back and try again."; - PutFooter(); - exit(); -} - -foreach my $id (@buglist) { - $::FORM{$id} = trim($::FORM{$id}); - if ($::FORM{$id} !~ /\d+/ || $::FORM{$id} < 0) { - PutHeader("Numbers only, please"); - print "Only use numeric values for your bug votes.\n"; - print "Please click Back and try again.
\n"; - PutFooter(); - exit(); +# If the user is voting for bugs, make sure they aren't overstuffing +# the ballot box. +if (scalar(@buglist)) { + SendSQL("SELECT bugs.bug_id, bugs.product, products.maxvotesperbug " . + "FROM bugs, products " . + "WHERE products.product = bugs.product ". + " AND bugs.bug_id IN (" . join(", ", @buglist) . ")"); + + my %prodcount; + + while (MoreSQLData()) { + my ($id, $prod, $max) = (FetchSQLData()); + if (!defined $prodcount{$prod}) { + $prodcount{$prod} = 0; + } + $prodcount{$prod} += $::FORM{$id}; + if ($::FORM{$id} > $max) { + PutHeader("Don't overstuff!", "Illegal vote"); + print "You may only use at most $max votes for a single bug in the\n"; + print "$prod product, but you are using $::FORM{$id}.\n"; + print "

Please click Back and try again.


\n"; + PutFooter(); + exit(); + } } -} - -SendSQL("SELECT bugs.bug_id, bugs.product, products.maxvotesperbug " . - "FROM bugs, products " . - "WHERE products.product = bugs.product ". - " AND bugs.bug_id IN (" . join(", ", @buglist) . ")"); - -my %prodcount; - -while (MoreSQLData()) { - my ($id, $prod, $max) = (FetchSQLData()); - if (!defined $prodcount{$prod}) { - $prodcount{$prod} = 0; - } - $prodcount{$prod} += $::FORM{$id}; - if ($::FORM{$id} > $max) { - PutHeader("Don't overstuff!", "Illegal vote"); - print "You may only use at most $max votes for a single bug in the\n"; - print "$prod product, but you are using $::FORM{$id}.\n"; - print "

Please click Back and try again.


\n"; - PutFooter(); - exit(); - } -} -foreach my $prod (keys(%prodcount)) { - if ($prodcount{$prod} > $::prodmaxvotes{$prod}) { - PutHeader("Don't overstuff!", "Illegal vote"); - print "You may only use $::prodmaxvotes{$prod} votes for bugs in the\n"; - print "$prod product, but you are using $prodcount{$prod}.\n"; - print "

Please click Back and try again.


\n"; - PutFooter(); - exit(); + foreach my $prod (keys(%prodcount)) { + if ($prodcount{$prod} > $::prodmaxvotes{$prod}) { + PutHeader("Don't overstuff!", "Illegal vote"); + print "You may only use $::prodmaxvotes{$prod} votes for bugs in the\n"; + print "$prod product, but you are using $prodcount{$prod}.\n"; + print "

Please click Back and try again.


\n"; + PutFooter(); + exit(); + } } } +# Update the user's votes in the database. If the user did not submit +# any votes, they may be using a form with checkboxes to remove all their +# votes (checkboxes are not submitted along with other form data when +# they are not checked, and Bugzilla uses them to represent single votes +# for products that only allow one vote per bug). In that case, we still +# need to clear the user's votes from the database. my %affected; SendSQL("lock tables bugs write, votes write"); SendSQL("select bug_id from votes where who = $who"); -- cgit v1.2.3-24-g4f1b