Closed
Bug 507389
Opened 15 years ago
Closed 15 years ago
[SECURITY] Users can see all products when editing bugs
Categories
(Bugzilla :: Creating/Changing Bugs, defect, P1)
Tracking
()
RESOLVED
FIXED
Bugzilla 3.4
People
(Reporter: pupykin.s, Assigned: LpSolit)
References
Details
(Keywords: regression)
Attachments
(1 file, 3 obsolete files)
3.08 KB,
patch
|
mkanat
:
review+
|
Details | Diff | Splinter Review |
User-Agent: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9.1.1) Gecko/20090718 Shiretoko/3.5.1 Build Identifier: bugzilla-3.4 User can see all products even he has no access to them. Reproducible: Always Steps to Reproduce: 1. create 2 products: prod1 and prod2 2. restrict user1 access only to prod1 3. try to edit own bug and see all products in combobox Actual Results: user can see full product list Expected Results: user can see allowed product list product list page works ok. problem affects only combo and listboxes on new chart and editbug pages
Assignee | ||
Comment 1•15 years ago
|
||
I can reproduce on tip and 3.4. I cannot reproduce on 3.2.4 and 3.0.8, so this is a regression. To be exact: a product must be totally hidden if group settings are Mandatory/Mandatory + the Entry bit is set.
Group: bugzilla-security
Severity: normal → critical
Status: UNCONFIRMED → NEW
Ever confirmed: true
Flags: blocking3.4.1+
Keywords: regression
OS: Linux → All
Priority: -- → P1
Hardware: x86 → All
Target Milestone: --- → Bugzilla 3.4
Version: unspecified → 3.4
Assignee | ||
Updated•15 years ago
|
Severity: critical → major
Assignee | ||
Comment 2•15 years ago
|
||
This is a regression due to bug 371995, which replaced bug.choices.product (which correctly calls $user->get_enterable_products) by Bugzilla::Product->get_all. Bugzilla 3.3.4 and newer are affected.
Depends on: 371995
Version: 3.4 → 3.3.4
Assignee | ||
Comment 3•15 years ago
|
||
We need to pass the list of products to use.
Assignee: create-and-change → LpSolit
Status: NEW → ASSIGNED
Attachment #391655 -
Flags: review?(mkanat)
Assignee | ||
Comment 4•15 years ago
|
||
The problem related to charts is different, as charts do not see categories as products.
Summary: user can see all products if he edits bug or creates new chart → [SECURITY] Users can see all products when editing bugs
Updated•15 years ago
|
Attachment #391655 -
Flags: review?(mkanat) → review-
Comment 5•15 years ago
|
||
Comment on attachment 391655 [details] [diff] [review] patch, v1 >Index: template/en/default/bug/edit.html.tmpl > <tr> >+ [% prod_list = user.get_enterable_products %] >+ [% IF NOT user.can_enter_product(bug.product) %] >+ [% prod_list.push(bug.product_obj) %] >+ [% prod_list = prod_list.sort('name') %] You shouldn't need to sort them by name again--Perl will sort them differently than the database will, so it's best to just let the DB sort them. >Index: template/en/default/bug/field.html.tmpl >+ # list (optional): The list of legal values, for select fields. Hmm. In other patches/customizations, I've called this "override_legal_values", which I think is better, because "list" could be a very common variable name.
Updated•15 years ago
|
Severity: major → critical
Assignee | ||
Comment 6•15 years ago
|
||
(In reply to comment #5) > You shouldn't need to sort them by name again--Perl will sort them > differently than the database will, so it's best to just let the DB sort them. I have to sort the list here because the added product shouldn't be at the end (unless we are fine with this, but this is confusing).
Comment 7•15 years ago
|
||
Um, I'd be okay with having the extra product at the *beginning*, I think.
Assignee | ||
Comment 8•15 years ago
|
||
(In reply to comment #7) > Um, I'd be okay with having the extra product at the *beginning*, I think. OK, sure, easy to do. Patch coming.
Assignee | ||
Comment 9•15 years ago
|
||
I now only set the override_... list if the user can edit the product field, else we don't waste time generating the list as the field would be read-only.
Attachment #391655 -
Attachment is obsolete: true
Attachment #391843 -
Flags: review?(mkanat)
Updated•15 years ago
|
Attachment #391843 -
Flags: review?(mkanat) → review+
Comment 10•15 years ago
|
||
Comment on attachment 391843 [details] [diff] [review] patch, v2 >Index: template/en/default/bug/edit.html.tmpl >=================================================================== >RCS file: /cvsroot/mozilla/webtools/bugzilla/template/en/default/bug/edit.html.tmpl,v >retrieving revision 1.160 >diff -3 -p -u -r1.160 edit.html.tmpl >--- template/en/default/bug/edit.html.tmpl 23 Jul 2009 06:36:10 -0000 1.160 >+++ template/en/default/bug/edit.html.tmpl 31 Jul 2009 10:16:52 -0000 >@@ -375,8 +375,16 @@ > [%#############%] > > <tr> >+ [% IF bug.check_can_change_field('product', 0, 1) %] >+ [% prod_list = user.get_enterable_products %] >+ [% IF NOT user.can_enter_product(bug.product) %] >+ [% prod_list.unshift(bug.product_obj) %] >+ [% END %] >+ [% END %] >+ > [% INCLUDE bug/field.html.tmpl > bug = bug, field = select_fields.product, >+ override_legal_values = prod_list > desc_url = 'describecomponents.cgi', value = bug.product > editable = bug.check_can_change_field('product', 0, 1) %] > </tr> >Index: template/en/default/bug/field.html.tmpl >=================================================================== >RCS file: /cvsroot/mozilla/webtools/bugzilla/template/en/default/bug/field.html.tmpl,v >retrieving revision 1.29 >diff -3 -p -u -r1.29 field.html.tmpl >--- template/en/default/bug/field.html.tmpl 17 Jul 2009 22:40:15 -0000 1.29 >+++ template/en/default/bug/field.html.tmpl 31 Jul 2009 10:16:52 -0000 >@@ -23,6 +23,7 @@ > [%# INTERFACE: > # field: a Bugzilla::Field object > # value: The value of the field for this bug. >+ # override_legal_values (optional): The list of legal values, for select fields. > # editable: Whether the field should be displayed as an editable > # <input> or as just the plain text of its value. > # allow_dont_change: display the --do_not_change-- option for select fields. >@@ -130,7 +131,10 @@ > [% dontchange FILTER html %] > </option> > [% END %] >- [% FOREACH legal_value = field.legal_values %] >+ [% IF NOT override_legal_values %] >+ [% override_legal_values = field.legal_values %] >+ [% END %] >+ [% FOREACH legal_value = override_legal_values %] > [% SET control_value = legal_value.visibility_value %] > [% SET control_field = field.value_field %] > <option value="[% legal_value.name FILTER html %]"
Comment 11•15 years ago
|
||
We should release ASAP to get out a fix for this issue.
Flags: approval?
Flags: approval3.4?
Assignee | ||
Comment 12•15 years ago
|
||
I just realized my patch doesn't pass 008filter.t, because unshift() is not in the whitelist. Fixed.
Attachment #391843 -
Attachment is obsolete: true
Attachment #392078 -
Flags: review?(mkanat)
Updated•15 years ago
|
Attachment #392078 -
Flags: review?(mkanat) → review+
Assignee | ||
Updated•15 years ago
|
Flags: approval?
Flags: approval3.4?
Flags: approval3.4+
Flags: approval+
Assignee | ||
Comment 13•15 years ago
|
||
tip: Checking in t/008filter.t; /cvsroot/mozilla/webtools/bugzilla/t/008filter.t,v <-- 008filter.t new revision: 1.30; previous revision: 1.29 done Checking in template/en/default/bug/edit.html.tmpl; /cvsroot/mozilla/webtools/bugzilla/template/en/default/bug/edit.html.tmpl,v <-- edit.html.tmpl new revision: 1.161; previous revision: 1.160 done Checking in template/en/default/bug/field.html.tmpl; /cvsroot/mozilla/webtools/bugzilla/template/en/default/bug/field.html.tmpl,v <-- field.html.tmpl new revision: 1.30; previous revision: 1.29 done 3.4: Checking in t/008filter.t; /cvsroot/mozilla/webtools/bugzilla/t/008filter.t,v <-- 008filter.t new revision: 1.29.2.1; previous revision: 1.29 done Checking in template/en/default/bug/edit.html.tmpl; /cvsroot/mozilla/webtools/bugzilla/template/en/default/bug/edit.html.tmpl,v <-- edit.html.tmpl new revision: 1.156.2.2; previous revision: 1.156.2.1 done Checking in template/en/default/bug/field.html.tmpl; /cvsroot/mozilla/webtools/bugzilla/template/en/default/bug/field.html.tmpl,v <-- field.html.tmpl new revision: 1.24.2.4; previous revision: 1.24.2.3 done
Status: ASSIGNED → RESOLVED
Closed: 15 years ago
Resolution: --- → FIXED
Updated•15 years ago
|
Flags: testcase?
Flags: approval?
Flags: approval3.4?
Flags: approval3.4+
Flags: approval+
Updated•15 years ago
|
Flags: approval?
Flags: approval3.4?
Flags: approval3.4+
Flags: approval+
Assignee | ||
Comment 14•15 years ago
|
||
The previous patch has a bug where values can be overriden by other fields. This is a full patch, with 2.1 being backed out.
Attachment #392078 -
Attachment is obsolete: true
Attachment #392086 -
Flags: review?(mkanat)
Updated•15 years ago
|
Attachment #392086 -
Flags: review?(mkanat) → review+
Assignee | ||
Comment 15•15 years ago
|
||
tip: Checking in template/en/default/bug/field.html.tmpl; /cvsroot/mozilla/webtools/bugzilla/template/en/default/bug/field.html.tmpl,v <-- field.html.tmpl new revision: 1.31; previous revision: 1.30 done 3.4: Checking in template/en/default/bug/field.html.tmpl; /cvsroot/mozilla/webtools/bugzilla/template/en/default/bug/field.html.tmpl,v <-- field.html.tmpl new revision: 1.24.2.5; previous revision: 1.24.2.4 done
You need to log in
before you can comment on or make changes to this bug.
Description
•