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

RESOLVED FIXED in Bugzilla 2.16

Status

()

Bugzilla
Creating/Changing Bugs
P1
blocker
RESOLVED FIXED
16 years ago
5 years ago

People

(Reporter: George Hotelling, Assigned: George Hotelling)

Tracking

2.14
Bugzilla 2.16

Details

(Whiteboard: applied to 2.14.1)

Attachments

(2 attachments, 5 obsolete attachments)

(Assignee)

Description

16 years ago
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.
(Assignee)

Updated

16 years ago
OS: Windows 2000 → OpenBSD
confirmed.  nifty, we missed one. :(
Severity: normal → critical
Priority: -- → P1
Whiteboard: security
Target Milestone: --- → Bugzilla 2.16
OS: OpenBSD → All
Hardware: PC → All
(Assignee)

Comment 2

16 years ago
Created attachment 51521 [details] [diff] [review]
A patch that seems to have fixed the problem on my installation.
(Assignee)

Comment 3

16 years ago
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.
George, can you attach a unified version of your patch?  ("cvs diff -u")
(Assignee)

Comment 5

16 years ago
Created attachment 51609 [details] [diff] [review]
Unified version of the patch

Updated

16 years ago
Keywords: patch
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
Severity: critical → blocker
Summary: When changing a bug the Product: list has options the user doesn't have access to. → [security] When changing a bug the Product: list has options the user doesn't have access to.
Created attachment 58650 [details] [diff] [review]
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.
Attachment #51521 - Attachment is obsolete: true
Attachment #51609 - Attachment is obsolete: true
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.
Attachment #58650 - Flags: review+
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.
Attachment #58650 - Flags: review+ → review-
Created attachment 58658 [details] [diff] [review]
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.
Attachment #58650 - Attachment is obsolete: true
Created attachment 58659 [details] [diff] [review]
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)
Attachment #58658 - Attachment is obsolete: true
Created attachment 58660 [details] [diff] [review]
Patch v4 - adds a comment I forgot. :)
Attachment #58659 - Attachment is obsolete: true
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.
Attachment #58660 - Flags: review+
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
Attachment #58660 - Flags: review+
Re-assigning to Dave since it's his patch.
Assignee: myk → justdave
reassigning to the original author so we have a contributor record
Assignee: justdave → george.hotelling
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
Status: NEW → RESOLVED
Last Resolved: 16 years ago
Resolution: --- → FIXED
Whiteboard: security → applied to 2.14.1
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.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Created attachment 60249 [details] [diff] [review]
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 on attachment 60249 [details] [diff] [review]
Fix v.1 to Patch v.4

pretty obvious :)
r=justdave
Attachment #60249 - Flags: review+
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.
Attachment #60249 - Flags: review+
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
Status: REOPENED → RESOLVED
Last Resolved: 16 years ago16 years ago
Resolution: --- → FIXED
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

16 years ago
*** Bug 126363 has been marked as a duplicate of this bug. ***
QA Contact: matty_is_a_geek → default-qa
You need to log in before you can comment on or make changes to this bug.