diff options
| -rw-r--r-- | CGI.pl | 49 | ||||
| -rwxr-xr-x | process_bug.cgi | 70 | ||||
| -rwxr-xr-x | showdependencygraph.cgi | 26 | ||||
| -rwxr-xr-x | showdependencytree.cgi | 23 | ||||
| -rwxr-xr-x | showvotes.cgi | 61 |
5 files changed, 160 insertions, 69 deletions
@@ -226,6 +226,55 @@ sub CheckFormFieldDefined (\%$) { } } +sub ValidateBugID { + # Validates and verifies a bug ID, making sure the number is a + # positive integer, that it represents an existing bug in the + # database, and that the user is authorized to access that bug. + + my ($id) = @_; + + # Make sure the bug number is a positive integer. + $id =~ /^([1-9][0-9]*)$/ + || DisplayError("The bug number is invalid.") + && exit; + + # Make sure the usergroupset variable is set. This variable stores + # the set of groups the user is a member of. This variable should + # be set by either confirm_login or quietly_check_login, but we set + # it here just in case one of those functions has not been run yet. + $::usergroupset ||= 0; + + # Query the database for the bug, retrieving a boolean value that + # represents whether or not the user is authorized to access the bug. + + # Users are authorized to access bugs if they are a member of all + # groups to which the bug is restricted. User group membership and + # bug restrictions are stored as bits within bitsets, so authorization + # can be determined by comparing the intersection of the user's + # bitset with the bug's bitset. If the result matches the bug's bitset + # the user is a member of all groups to which the bug is restricted + # and is authorized to access the bug. + + # Bit arithmetic is performed by MySQL instead of Perl because bitset + # fields in the database are 64 bits wide (BIGINT), and Perl installations + # may or may not support integers larger than 32 bits. Using bitsets + # and doing bitset arithmetic is probably not cross-database compatible, + # however, so these mechanisms are likely to change in the future. + SendSQL("SELECT ((groupset & $::usergroupset) = groupset) + FROM bugs WHERE bug_id = $id"); + + # Make sure the bug exists in the database. + MoreSQLData() + || DisplayError("Bug #$id does not exist.") + && exit; + + # Make sure the user is authorized to access the bug. + my ($isauthorized) = FetchSQLData(); + $isauthorized + || DisplayError("You are not authorized to access bug #$id.") + && exit; +} + # check and see if a given string actually represents a positive # integer, and abort if not. # diff --git a/process_bug.cgi b/process_bug.cgi index dded85dbb..4b4453dc1 100755 --- a/process_bug.cgi +++ b/process_bug.cgi @@ -48,6 +48,35 @@ my $whoid = confirm_login(); my $requiremilestone = 0; +###################################################################### +# Begin Data/Security Validation +###################################################################### + +# Create a list of IDs of all bugs being modified in this request. +# This list will either consist of a single bug number from the "id" +# form/URL field or a series of numbers from multiple form/URL fields +# named "id_x" where "x" is the bug number. +my @idlist; +if (defined $::FORM{'id'}) { + push @idlist, $::FORM{'id'}; +} else { + foreach my $i (keys %::FORM) { + if ($i =~ /^id_([1-9][0-9]*)/) { + push @idlist, $1; + } + } +} + +# For each bug being modified, make sure its ID is a valid bug number +# representing an existing bug that the user is authorized to access. +foreach my $id (@idlist) { + ValidateBugID($id); +} + +###################################################################### +# End Data/Security Validation +###################################################################### + print "Content-type: text/html\n\n"; PutHeader ("Bug processed"); @@ -221,9 +250,7 @@ empowered user, may make that change to the $f field. -my @idlist; -if (defined $::FORM{'id'}) { - +if (defined $::FORM{'id'} && Param('strictvaluechecks')) { # since this means that we were called from show_bug.cgi, now is a good # time to do a whole bunch of error checking that can't easily happen when # we've been called from buglist.cgi, because buglist.cgi only tweaks @@ -231,31 +258,18 @@ if (defined $::FORM{'id'}) { # (XXX those error checks need to happen too, but implementing them # is more work in the current architecture of this script...) # - if ( Param('strictvaluechecks') ) { - CheckFormField(\%::FORM, 'rep_platform', \@::legal_platform); - CheckFormField(\%::FORM, 'priority', \@::legal_priority); - CheckFormField(\%::FORM, 'bug_severity', \@::legal_severity); - CheckFormField(\%::FORM, 'component', - \@{$::components{$::FORM{'product'}}}); - CheckFormFieldDefined(\%::FORM, 'bug_file_loc'); - CheckFormFieldDefined(\%::FORM, 'short_desc'); - CheckFormField(\%::FORM, 'product', \@::legal_product); - CheckFormField(\%::FORM, 'version', - \@{$::versions{$::FORM{'product'}}}); - CheckFormField(\%::FORM, 'op_sys', \@::legal_opsys); - CheckFormFieldDefined(\%::FORM, 'longdesclength'); - CheckPosInt($::FORM{'id'}); - } - push @idlist, $::FORM{'id'}; -} else { - foreach my $i (keys %::FORM) { - if ($i =~ /^id_/) { - if ( Param('strictvaluechecks') ) { - CheckPosInt(substr($i, 3)); - } - push @idlist, substr($i, 3); - } - } + CheckFormField(\%::FORM, 'rep_platform', \@::legal_platform); + CheckFormField(\%::FORM, 'priority', \@::legal_priority); + CheckFormField(\%::FORM, 'bug_severity', \@::legal_severity); + CheckFormField(\%::FORM, 'component', + \@{$::components{$::FORM{'product'}}}); + CheckFormFieldDefined(\%::FORM, 'bug_file_loc'); + CheckFormFieldDefined(\%::FORM, 'short_desc'); + CheckFormField(\%::FORM, 'product', \@::legal_product); + CheckFormField(\%::FORM, 'version', + \@{$::versions{$::FORM{'product'}}}); + CheckFormField(\%::FORM, 'op_sys', \@::legal_opsys); + CheckFormFieldDefined(\%::FORM, 'longdesclength'); } my $action = ''; diff --git a/showdependencygraph.cgi b/showdependencygraph.cgi index f15534be3..df377c096 100755 --- a/showdependencygraph.cgi +++ b/showdependencygraph.cgi @@ -25,8 +25,28 @@ use strict; require "CGI.pl"; +ConnectToDatabase(); + +quietly_check_login(); + +$::usergroupset = $::usergroupset; # More warning suppression silliness. + +###################################################################### +# Begin Data/Security Validation +###################################################################### + +# Make sure the bug ID is a positive integer representing an existing +# bug that the user is authorized to access. +if (defined $::FORM{'id'}) { + ValidateBugID($::FORM{'id'}); +} + +###################################################################### +# End Data/Security Validation +###################################################################### + my $id = $::FORM{'id'}; -die "Invalid id: $id" unless $id =~ /^\s*\d+\s*$/; + my $urlbase = Param("urlbase"); my %seen; @@ -51,10 +71,6 @@ $::FORM{'rankdir'} = "LR" if !defined $::FORM{'rankdir'}; if (defined $id) { - ConnectToDatabase(); - quietly_check_login(); - $::usergroupset = $::usergroupset; # More warning suppression silliness. - mkdir("data/webdot", 0777); my $filename = "data/webdot/$$.dot"; diff --git a/showdependencytree.cgi b/showdependencytree.cgi index 74e2778bc..bab36da61 100755 --- a/showdependencytree.cgi +++ b/showdependencytree.cgi @@ -29,6 +29,23 @@ require "CGI.pl"; use vars %::FORM; +ConnectToDatabase(); + +quietly_check_login(); + +$::usergroupset = $::usergroupset; # More warning suppression silliness. + +###################################################################### +# Begin Data/Security Validation +###################################################################### + +# Make sure the bug ID is a positive integer representing an existing +# bug that the user is authorized to access. +ValidateBugID($::FORM{'id'}); + +###################################################################### +# End Data/Security Validation +###################################################################### my $id = $::FORM{'id'}; my $linkedid = qq{<a href="show_bug.cgi?id=$id">$id</a>}; @@ -36,12 +53,6 @@ my $linkedid = qq{<a href="show_bug.cgi?id=$id">$id</a>}; print "Content-type: text/html\n\n"; PutHeader("Dependency tree", "Dependency tree", "Bug $linkedid"); -ConnectToDatabase(); - -quietly_check_login(); - -$::usergroupset = $::usergroupset; # More warning suppression silliness. - my %seen; sub DumpKids { diff --git a/showvotes.cgi b/showvotes.cgi index 575156786..bb87848f0 100755 --- a/showvotes.cgi +++ b/showvotes.cgi @@ -28,50 +28,51 @@ require "CGI.pl"; ConnectToDatabase(); +if (defined $::FORM{'voteon'} || (!defined $::FORM{'bug_id'} && + !defined $::FORM{'user'})) { + confirm_login(); + $::FORM{'user'} = DBNameToIdAndCheck($::COOKIE{'Bugzilla_login'}); +} else { + # Check whether or not the user is currently logged in without throwing + # an error if the user is not logged in. This function sets the value + # of $::usergroupset, the binary number that records the set of groups + # to which the user belongs and which gets used in ValidateBugID below + # to determine whether or not the user is authorized to access the bug + # whose votes are being shown or which is being voted on. + quietly_check_login(); +} + ################################################################################ -# START Form Data Validation +# Begin Data/Security Validation ################################################################################ -# For security and correctness, validate the value of the "voteon" form variable. -# Valid values are those containing a number that is the ID of an existing bug. -if (defined $::FORM{'voteon'}) { - $::FORM{'voteon'} =~ /^(\d+)$/; - $::FORM{'voteon'} = $1 || 0; - SendSQL("SELECT bug_id FROM bugs WHERE bug_id = $::FORM{'voteon'}"); - FetchSQLData() - || DisplayError("You entered an invalid bug number to vote on.") && exit; +# Make sure the bug ID is a positive integer representing an existing +# bug that the user is authorized to access. +if (defined $::FORM{'bug_id'}) { + ValidateBugID($::FORM{'bug_id'}); } -# For security and correctness, validate the value of the "bug_id" form variable. -# Valid values are those containing a number that is the ID of an existing bug. -if (defined $::FORM{'bug_id'}) { - $::FORM{'bug_id'} =~ /^(\d+)$/; - $::FORM{'bug_id'} = $1 || 0; - SendSQL("SELECT bug_id FROM bugs WHERE bug_id = $::FORM{'bug_id'}"); - FetchSQLData() - || DisplayError("You entered an invalid bug number.") && exit; +# Make sure the bug ID being voted on is a positive integer representing +# an existing bug that the user is authorized to access. +if (defined $::FORM{'voteon'}) { + ValidateBugID($::FORM{'voteon'}); } -# For security and correctness, validate the value of the "userid" form variable. -# Valid values are those containing a number that is the ID of an existing user. +# Make sure the user ID is a positive integer representing an existing user. if (defined $::FORM{'user'}) { - $::FORM{'user'} =~ /^(\d+)$/; - $::FORM{'user'} = $1 || 0; - SendSQL("SELECT userid FROM profiles WHERE userid = $::FORM{'user'}"); + $::FORM{'user'} =~ /^([1-9][0-9]*)$/ + || DisplayError("The user number is invalid.") + && exit; + SendSQL("SELECT 1 FROM profiles WHERE userid = $::FORM{'user'}"); FetchSQLData() - || DisplayError("You specified an invalid user number.") && exit; + || DisplayError("User #$::FORM{'user'} does not exist.") + && exit; } ################################################################################ -# END Form Data Validation +# End Data/Security Validation ################################################################################ -if (defined $::FORM{'voteon'} || (!defined $::FORM{'bug_id'} && - !defined $::FORM{'user'})) { - confirm_login(); - $::FORM{'user'} = DBNameToIdAndCheck($::COOKIE{'Bugzilla_login'}); -} - print "Content-type: text/html\n\n"; if (defined $::FORM{'bug_id'}) { |