aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authormkanat%kerio.com <>2005-05-12 09:07:09 +0000
committermkanat%kerio.com <>2005-05-12 09:07:09 +0000
commite2252835e8e96371d6536af5dbd72a79e6ed05b5 (patch)
treec76c89f4a2fc3e7c0e9172efd988d8d49c0c4e5f
parent8f2bc1b07ce4150a878e80f5bce09e819cbfd414 (diff)
downloadbugs-e2252835e8e96371d6536af5dbd72a79e6ed05b5.tar
bugs-e2252835e8e96371d6536af5dbd72a79e6ed05b5.tar.gz
bugs-e2252835e8e96371d6536af5dbd72a79e6ed05b5.tar.bz2
bugs-e2252835e8e96371d6536af5dbd72a79e6ed05b5.tar.xz
bugs-e2252835e8e96371d6536af5dbd72a79e6ed05b5.zip
Bug 287109: [SECURITY] Names of private products/components can be exposed on certain CGIs
Patch By Frederic Buclin <LpSolit@gmail.com> r=myk, r=joel, a=justdave
-rwxr-xr-xenter_bug.cgi5
-rw-r--r--globals.pl64
-rwxr-xr-xpost_bug.cgi7
-rwxr-xr-xprocess_bug.cgi42
-rwxr-xr-xreports.cgi4
-rw-r--r--template/en/default/global/user-error.html.tmpl5
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