From: mkanat%kerio.com <> Date: Thu, 12 May 2005 09:08:34 +0000 (+0000) Subject: Bug 287109: [SECURITY] Names of private products/components can be exposed on certain... X-Git-Tag: bugzilla-2.18.1~1 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=2de0dda74d14a5c9bd57ea25235e1f5d648d6924;p=thirdparty%2Fbugzilla.git Bug 287109: [SECURITY] Names of private products/components can be exposed on certain CGIs Patch By Frederic Buclin r=myk, a=justdave --- diff --git a/enter_bug.cgi b/enter_bug.cgi index 0b30f65e69..d41ea4fd02 100755 --- a/enter_bug.cgi +++ b/enter_bug.cgi @@ -223,17 +223,10 @@ Bugzilla->login(LOGIN_REQUIRED) if (!(AnyEntryGroups())); # 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(); -if (lsearch(\@::enterable_products, $product) == -1) { - ThrowUserError("invalid_product_name", { product => $product}); -} - my $product_id = get_product_id($product); if (0 == @{$::components{$product}}) { diff --git a/globals.pl b/globals.pl index c4cb8d33a2..99a58daded 100644 --- a/globals.pl +++ b/globals.pl @@ -493,11 +493,16 @@ sub CanEditProductId { return (!defined($result)); } -# -# 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 " . @@ -507,12 +512,51 @@ sub CanEnterProduct { $query .= "AND group_id NOT IN(" . join(',', values(%{Bugzilla->user->groups})) . ") "; } - $query .= "WHERE products.name = " . SqlQuote($productname) . " LIMIT 1"; - PushGlobalSQLState(); - SendSQL($query); - my ($ret) = FetchSQLData(); - PopGlobalSQLState(); - return ($ret); + $query .= "WHERE products.name = ? LIMIT 1"; + + 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 = ? 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 56e57c5125..514e57a311 100755 --- a/post_bug.cgi +++ b/post_bug.cgi @@ -78,11 +78,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 = $::FORM{'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 3dd5f52ee3..61270c4675 100755 --- a/process_bug.cgi +++ b/process_bug.cgi @@ -45,7 +45,8 @@ use Bugzilla::Flag; # Shut up misguided -w warnings about "used only once": -use vars qw(%versions +use vars qw(@legal_product + %versions %components %COOKIE %legal_opsys @@ -247,8 +248,24 @@ if ((($::FORM{'id'} && $::FORM{'product'} ne $::oldproduct) "abort"); } - CheckFormField(\%::FORM, 'product', \@::legal_product); my $prod = $::FORM{'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) . ") LIMIT 1", + undef, $prod) || 0; + + 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 @@ -708,6 +725,7 @@ if ($::FORM{'component'} ne $::FORM{'dontchange'}) { product => $::FORM{'product'}}); DoComma(); + $::FORM{'component_id'} = $comp_id; $::query .= "component_id = $comp_id"; } @@ -1122,17 +1140,6 @@ foreach my $id (@idlist) { "group_control_map READ"); my @oldvalues = SnapShotBug($id); my %oldhash; - # 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($::FORM{'product'}); - - if ($::FORM{'component'} ne $::FORM{'dontchange'}) { - $::FORM{'component_id'} = - get_component_id($product_id, $::FORM{'component'}); - } my $i = 0; foreach my $col (@::log_columns) { @@ -1183,13 +1190,6 @@ foreach my $id (@idlist) { { product => $oldhash{'product'} }, "abort"); } - if (defined $::FORM{'product'} - && $::FORM{'product'} ne $::FORM{'dontchange'} - && $::FORM{'product'} ne $oldhash{'product'} - && !CanEnterProduct($::FORM{'product'})) { - ThrowUserError("entry_access_denied", - { product => $::FORM{'product'} }, "abort"); - } 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 a3e2c740e0..c5314b33e7 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 f0c94ef932..aaf53a2058 100644 --- a/template/en/default/global/user-error.html.tmpl +++ b/template/en/default/global/user-error.html.tmpl @@ -166,8 +166,9 @@ [% ELSIF error == "entry_access_denied" %] [% title = "Permission Denied" %] - Sorry; you do not have the permissions necessary to enter [% terms.abug %] - against the [% product FILTER html %] product. + Sorry, either the product [% product FILTER html %] does not + exist, or you don't have the required permissions to + enter [% terms.abug %] against that product. [% ELSIF error == "file_not_specified" %] [% title = "No File Specified" %] @@ -609,6 +610,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 [% product FILTER html %] has been disabled. + [% ELSIF error == "product_edit_denied" %] [% title = "Product Edit Access Denied" %] You are not permitted to edit [% terms.bugs %] in product