diff options
-rwxr-xr-x | enter_bug.cgi | 5 | ||||
-rw-r--r-- | globals.pl | 64 | ||||
-rwxr-xr-x | post_bug.cgi | 7 | ||||
-rwxr-xr-x | process_bug.cgi | 42 | ||||
-rwxr-xr-x | reports.cgi | 4 | ||||
-rw-r--r-- | template/en/default/global/user-error.html.tmpl | 5 |
6 files changed, 86 insertions, 41 deletions
diff --git a/enter_bug.cgi b/enter_bug.cgi index 924977a3e..f3437b7ed 100755 --- a/enter_bug.cgi +++ b/enter_bug.cgi @@ -292,10 +292,7 @@ if ($cloned_bug_id) { # We need to check and make sure # that the user has permission to enter a bug against this product. -if(!CanEnterProduct($product)) -{ - ThrowUserError("entry_access_denied", { product => $product}); -} +CanEnterProductOrWarn($product); GetVersionTable(); diff --git a/globals.pl b/globals.pl index d0e819f02..009f93ee9 100644 --- a/globals.pl +++ b/globals.pl @@ -436,12 +436,16 @@ sub IsInClassification { } } -# -# This function determines if a user can enter bugs in the named -# product. +# This function determines whether or not a user can enter +# bugs into the named product. sub CanEnterProduct { - my ($productname) = @_; + my ($productname, $verbose) = @_; my $dbh = Bugzilla->dbh; + + return unless defined($productname); + trick_taint($productname); + + # First check whether or not the user has access to that product. my $query = "SELECT group_id IS NULL " . "FROM products " . "LEFT JOIN group_control_map " . @@ -451,13 +455,53 @@ sub CanEnterProduct { $query .= "AND group_id NOT IN(" . join(',', values(%{Bugzilla->user->groups})) . ") "; } - $query .= "WHERE products.name = " . SqlQuote($productname) . " " . + $query .= "WHERE products.name = ? " . $dbh->sql_limit(1); - PushGlobalSQLState(); - SendSQL($query); - my ($ret) = FetchSQLData(); - PopGlobalSQLState(); - return ($ret); + + my $has_access = $dbh->selectrow_array($query, undef, $productname); + if (!$has_access) { + # Do we require the exact reason why we cannot enter + # bugs into that product? Returning -1 explicitely + # means the user has no access to the product or the + # product does not exist. + return (defined($verbose)) ? -1 : 0; + } + + # Check if the product is open for new bugs and has + # at least one component. + my $allow_new_bugs = + $dbh->selectrow_array("SELECT CASE WHEN disallownew = 0 THEN 1 ELSE 0 END + FROM products INNER JOIN components + ON components.product_id = products.id + WHERE products.name = ? " . + $dbh->sql_limit(1), + undef, $productname); + + # Return 1 if the user can enter bugs into that product; + # return 0 if the product is closed for new bug entry; + # return undef if the product has no component. + return $allow_new_bugs; +} + +# Call CanEnterProduct() and display an error message +# if the user cannot enter bugs into that product. +sub CanEnterProductOrWarn { + my ($product) = @_; + + if (!defined($product)) { + ThrowUserError("no_products"); + } + my $status = CanEnterProduct($product, 1); + trick_taint($product); + + if (!defined($status)) { + ThrowUserError("no_components", { product => $product}); + } elsif (!$status) { + ThrowUserError("product_disabled", { product => $product}); + } elsif ($status < 0) { + ThrowUserError("entry_access_denied", { product => $product}); + } + return $status; } sub GetEnterableProducts { diff --git a/post_bug.cgi b/post_bug.cgi index 9a4860409..b9d63b3fe 100755 --- a/post_bug.cgi +++ b/post_bug.cgi @@ -79,11 +79,10 @@ $template->process($format->{'template'}, $vars, \$comment) ValidateComment($comment); # Check that the product exists and that the user -# is allowed to submit bugs in this product. +# is allowed to enter bugs into this product. my $product = $cgi->param('product'); -if (!CanEnterProduct($product)) { - ThrowUserError("entry_access_denied", {product => $product}); -} +CanEnterProductOrWarn($product); + my $product_id = get_product_id($product); # Set cookies diff --git a/process_bug.cgi b/process_bug.cgi index 451613e29..c000e3a4a 100755 --- a/process_bug.cgi +++ b/process_bug.cgi @@ -60,7 +60,8 @@ use Bugzilla::FlagType; # Shut up misguided -w warnings about "used only once": -use vars qw(%versions +use vars qw(@legal_product + %versions %components %legal_opsys %legal_platform @@ -268,9 +269,26 @@ if (((defined $cgi->param('id') && $cgi->param('product') ne $oldproduct) $vars->{'privs'} = $PrivilegesRequired; ThrowUserError("illegal_change", $vars); } - - CheckFormField($cgi, 'product', \@::legal_product); + my $prod = $cgi->param('product'); + trick_taint($prod); + + # If at least one bug does not belong to the product we are + # moving to, we have to check whether or not the user is + # allowed to enter bugs into that product. + # Note that this check must be done early to avoid the leakage + # of component, version and target milestone names. + my $check_can_enter = + $dbh->selectrow_array("SELECT 1 FROM bugs + INNER JOIN products + ON bugs.product_id = products.id + WHERE products.name != ? + AND bugs.bug_id IN + (" . join(',', @idlist) . ") " . + $dbh->sql_limit(1), + undef, $prod); + + if ($check_can_enter) { CanEnterProductOrWarn($prod) } # note that when this script is called from buglist.cgi (rather # than show_bug.cgi), it's possible that the product will be changed @@ -755,6 +773,7 @@ if ($cgi->param('component') ne $cgi->param('dontchange')) { {name => $cgi->param('component'), product => $cgi->param('product')}); + $cgi->param('component_id', $comp_id); DoComma(); $::query .= "component_id = $comp_id"; } @@ -1164,17 +1183,6 @@ foreach my $id (@idlist) { "group_control_map AS oldcontrolmap READ", "group_control_map AS newcontrolmap READ", "group_control_map READ", "email_setting READ"); - # Fun hack. @::log_columns only contains the component_id, - # not the name (since bug 43600 got fixed). So, we need to have - # this id ready for the loop below, otherwise anybody can - # change the component of a bug (we checked product above). - # http://bugzilla.mozilla.org/show_bug.cgi?id=180545 - my $product_id = get_product_id($cgi->param('product')); - - if ($cgi->param('component') ne $cgi->param('dontchange')) { - $cgi->param('component_id', - get_component_id($product_id, $cgi->param('component'))); - } # It may sound crazy to set %formhash for each bug as $cgi->param() # will not change, but %formhash is modified below and we prefer @@ -1258,12 +1266,6 @@ foreach my $id (@idlist) { { product => $oldhash{'product'} }); } - if ($cgi->param('product') ne $cgi->param('dontchange') - && $cgi->param('product') ne $oldhash{'product'} - && !CanEnterProduct($cgi->param('product'))) { - ThrowUserError("entry_access_denied", - { product => $cgi->param('product') }); - } if ($requiremilestone) { # musthavemilestoneonaccept applies only if at least two # target milestones are defined for the current product. diff --git a/reports.cgi b/reports.cgi index a3e2c740e..c5314b33e 100755 --- a/reports.cgi +++ b/reports.cgi @@ -85,9 +85,7 @@ if (! defined $cgi->param('product')) { # We don't want people to be able to view # reports for products they don't have permissions for... - if (($product ne '-All-') && (!CanEnterProduct($product))) { - ThrowUserError("report_access_denied"); - } + if ($product ne '-All-') { CanEnterProductOrWarn($product) } # We've checked that the product exists, and that the user can see it # This means that is OK to detaint diff --git a/template/en/default/global/user-error.html.tmpl b/template/en/default/global/user-error.html.tmpl index b7cefa9a3..f2ccb1497 100644 --- a/template/en/default/global/user-error.html.tmpl +++ b/template/en/default/global/user-error.html.tmpl @@ -920,6 +920,11 @@ Patches cannot be more than [% Param('maxpatchsize') %] KB in size. Try breaking your patch into several pieces. + [% ELSIF error == "product_disabled" %] + [% title = BLOCK %]Product closed for [% terms.Bugs %] Entry[% END %] + Sorry, entering [% terms.bugs %] into + product <em>[% product FILTER html %]</em> has been disabled. + [% ELSIF error == "product_edit_denied" %] [% title = "Product Edit Access Denied" %] You are not permitted to edit [% terms.bugs %] in product |