102141 - [security] When changing a bug the Product: list has options the user doesn't have access to.

Status: RESOLVED FIXED

(Bugzilla :: Creating/Changing Bugs, defect, P1)

Description

[George Hotelling]

If a bug is created in the product Foo, a user with Foo Bug Access will be able 
to see other products in the Product pulldown menu in show_bug.cgi.  I can 
reproduce it by doing the following:

1. Create a user Alice who has only Foo Bugs Access, and no other permissions 
checked.
2. Alice submits a bug for the Foo product.
3. Alice views the bug at show_bug.cgi
4. Alice selects the Product pulldown menu (normally used to change which 
product a bug belongs to) and sees that products Bar and Baz also exist.

Instead, Bugzilla should only display products that Alice has access to.  In 
this case the only Foo should be displayed.

Comment 1

[Dave Miller [:justdave]]

confirmed.  nifty, we missed one. :(

Comment 2

[George Hotelling]

Attachment: A patch that seems to have fixed the problem on my installation.

Comment 3

[George Hotelling]

I took most of the code for my patch from enter_bug.cgi.  This is my first time 
playing with the bugzilla code so please double or triple check my work.

Comment 4

[Myk Melez [:myk] [@mykmelez]]

George, can you attach a unified version of your patch?  ("cvs diff -u")

Comment 5

[George Hotelling]

Attachment: Unified version of the patch

Comment 6

[Gervase Markham [:gerv]]

We should factor out the common code. 

My idea: wait for the enter_bug.cgi templatisation to land and then make a patch
factoring it out into some common file.

Gerv

Comment 7

[Dave Miller [:justdave]]

Attachment: same as previous, but applies cleanly

This patch is identical to the one above, but it applies cleanly.  Not sure
what was wrong with the above one, but patch complained about garbage in the
file and wasn't able to determine what file was being patched.

Comment 8

[Dave Miller [:justdave]]

Comment on attachment 58650 [details] [diff] [review]
same as previous, but applies cleanly

r= justdave

it works as advertised, tried it on my local install.

Comment 9

[Dave Miller [:justdave]]

Comment on attachment 58650 [details] [diff] [review]
same as previous, but applies cleanly

I'm retracting my review...

scenario:

User does not have access to the product the bug is in.  Either the product bit
on the bug was turned off, or the cc_accessible bit is on and the user was CCed
on the bug.  If the user looks at the bug, it's not going to show the product
the bug is in, and it's also going to force the user to change the product to
something he does have access to if he tries to edit the bug.

It needs to add the current product to the @products list, even if the user
can't see it.

Comment 10

[Dave Miller [:justdave]]

Attachment: Patch v2 - include current product in popup

This one ensures that the product being checked isn't the current product
before removing it from consideration for the @product list.

Comment 11

[Dave Miller [:justdave]]

Attachment: Patch v3 - disallow new shouldn't be checked either for current product

This one, besides avoiding removing the product from the list if the product
the bug is already in has disallownew set, also redoes the patch to conform to
style guidelines (no tabs, 4-space indent, uncuddled else)

Comment 12

[Dave Miller [:justdave]]

Attachment: Patch v4 - adds a comment I forgot. :)

Comment 13

[Bradley Baetz (:bbaetz)]

Comment on attachment 58660 [details] [diff] [review]
Patch v4 - adds a comment I forgot. :)

r=bbaetz (finally :) I can't test this ATM, though, so the second reviewer will
have to.

Comment 14

[Christopher Aillon (sabbatical, not receiving bugmail)]

Comment on attachment 58660 [details] [diff] [review]
Patch v4 - adds a comment I forgot. :)

Code looks solid and it works great.  I tested it with a user on another bug,
and a user on the current bug. I also tested with disallownew set.

r=caillon

Comment 15

[Christopher Aillon (sabbatical, not receiving bugmail)]

Re-assigning to Dave since it's his patch.

Comment 16

[Dave Miller [:justdave]]

reassigning to the original author so we have a contributor record

Comment 17

[Dave Miller [:justdave]]

checked in on the trunk:

/cvsroot/mozilla/webtools/bugzilla/bug_form.pl,v  <--  bug_form.pl
new revision: 1.84; previous revision: 1.83

checked in on the 2.14.1 security release branch:

/cvsroot/mozilla/webtools/bugzilla/bug_form.pl,v  <--  bug_form.pl
new revision: 1.70.2.1; previous revision: 1.70

Comment 18

[David D. Kilzer (ddk)]

This patch created a bug when there is only one product defined.  No
"product" HTML form variable was set in show_bug.cgi, thus
process_bug.cgi would trip around line 139 since $::FORM{'product'}
woudl not equal $::oldproduct:

if ( ($::FORM{'id'} && $::FORM{'product'} ne $::oldproduct)
       || (!$::FORM{'id'} && $::FORM{'product'} ne $::dontchange) ) {

I will be attaching a small patch shortly that will fix this issue by
setting a hidden form variable named "product" when only one product
exists.

Comment 19

[David D. Kilzer (ddk)]

Attachment: Fix v.1 to Patch v.4

Patch to fix the issue when only one product is defined in Bugzilla.
This patch should be applied after Patch v.4 (attachment 58660 [details] [diff] [review]).

Comment 20

[Dave Miller [:justdave]]

Comment on attachment 60249 [details] [diff] [review]
Fix v.1 to Patch v.4

pretty obvious :)
r=justdave

Comment 21

[Bradley Baetz (:bbaetz)]

Comment on attachment 60249 [details] [diff] [review]
Fix v.1 to Patch v.4

r=bbaetz for 2.14.1 and trunk. Untested but obvious; justdave says he has
tested, though.

Comment 22

[Dave Miller [:justdave]]

Checked in on the trunk:

/cvsroot/mozilla/webtools/bugzilla/bug_form.pl,v  <--  bug_form.pl
new revision: 1.85; previous revision: 1.84

and on the 2.14.1 branch:

/cvsroot/mozilla/webtools/bugzilla/bug_form.pl,v  <--  bug_form.pl
new revision: 1.70.2.2; previous revision: 1.70.2.1

Comment 23

[Dave Miller [:justdave]]

Hmm, it seems the bulk change thinks I'm not changing anything if all I do is
add names to the CC list, so I guess I have to make a comment.  Anyhow, adding
the representatives from the organizations we know of that support Bugzilla
distributions so they're aware of our upcoming security release

Comment 24

[Daniel Schwager]

*** Bug 126363 has been marked as a duplicate of this bug. ***