209742 - describecomponents.cgi displays products for which the user can't enter bugs

Status: RESOLVED FIXED

(Bugzilla :: Bugzilla-General, defect)

Description

[Ryan Cleary]

User-Agent:       Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.3) Gecko/20030312
Build Identifier: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.3) Gecko/20030312

describecomponents.cgi uses AnyDefaultGroups() to determine whether to check
CanEnterProducts() before showing the list of available products.  If all
products are configured with Mandatory/Mandatory group controls,
AnyDefaultGroups() returns false, and the user is given a list of all products,
even ones they're not supposed to see.

I think describecomponents.cgi should use AnyEntryGroups() instead, so that
whenever bug entry groups are in use, only the products for which a user can
enter bugs are displayed (which is how I interpret the desired behavior from bug
185505).


Reproducible: Always

Steps to Reproduce:
1.
2.
3.

Comment 1

[timeless]

but what if the user is cc'd to some bug in some other product, and then wants a
description of the components in that product?

Comment 2

[Ryan Cleary]

Attachment: check CanEnterProduct() if AnyEntryGroups() is true

Comment 3

[Ryan Cleary]

I was only considering under what conditions describecomponents.cgi does the
CanEnterProduct() loop for displaying a list of produts, not what test(s)
determine whether the product is displayed.

If the user doesn't meet the CanEnterProduct() criteria, describecomponent.cgi
won't show him the components anyway (lines 90-92).  Should that be changed?

Comment 4

[Joel Peshkin]

Comment on attachment 125885 [details] [diff] [review]
check CanEnterProduct() if AnyEntryGroups() is true

r=joel

Being a security thing, I would like to see a 2xr  on this and confirmation
from the reporting site that it fixes the original problem.

The issue in comment 1 is a distinct issue.  In all the query forms and
decribecomponents, bugzilla has always used the product groups instead of
searching for any bug that is visible.	If that is something we wnt to change,
that would be a distinct bug.

Comment 5

[Ryan Cleary]

Re: comment 4

As far as I can tell, with my patch describecomponents.cgi only displays the
components of (or, with the additional fix below, acknowledges the existence of)
products that the user should be able to access.

I'm also going a step further, and throwing the "invalid bug" error whether the
bug is invalid or off-limits:

@@ -88,8 +88,12 @@
 }
 
 # Make sure the user is authorized to access this product.
+# If user is not authorized, give same error as if product
+# is invalid (as mentioned in preceeding comment).
 CanEnterProduct($product)
-      || ThrowUserError("product_access_denied");
+#      || ThrowUserError("product_access_denied");
+      || ThrowUserError("invalid_product_name",
+                   { product => $product });
 
 ######################################################################
 # End Data/Security Validation

but i appreciate that this could be a decision for the admin.

Comment 6

[Gervase Markham [:gerv]]

describecomponents.cgi should, IMO, display a list of exactly all products which
the user might be aware of legally through other means. This means products they
can view, and products they can enter bugs on.

I'm confused - is that what it does now, or not?

Gerv

Comment 7

[Joel Peshkin]

The bug is that there are conditions (specifically when the group controls are
mandatory and there are no default groups) where that is not true.

This patch makes that statement true.

Comment 1 suggests that we might take this one step further.

Comment 8

[Bradley Baetz (:bbaetz)]

Comment on attachment 125885 [details] [diff] [review]
check CanEnterProduct() if AnyEntryGroups() is true

r=bbaetz, and ISTR we have a separate bug on comment #1.

Comment 9

[Bradley Baetz (:bbaetz)]

Comment on attachment 125885 [details] [diff] [review]
check CanEnterProduct() if AnyEntryGroups() is true

r=bbaetz, and ISTR we have a separate bug on comment #1.

Comment 10

[Dave Miller [:justdave]]

er, hold that thought.  forgot this was a security bug...  we'll hold this till
we're ready to go on the release before we check it in.

I'm correct in thinking this has to do with the new groups architecture and so
it doesn't apply to the 2.16 branch, right?

Comment 11

[Dave Miller [:justdave]]

-> Patch author

Comment 12

[Dave Miller [:justdave]]

Checking in describecomponents.cgi;
/cvsroot/mozilla/webtools/bugzilla/describecomponents.cgi,v  <-- 
describecomponents.cgi
new revision: 1.23; previous revision: 1.22
done

Comment 13

[Dave Miller [:justdave]]

Security advisory has been posted.