Closed Bug 219690 Opened 16 years ago Closed 16 years ago

When deleting products and usebuggroups is set, blessgroupset is not updated

Categories

(Bugzilla :: Administration, task, major)

2.16.3
task
Not set
major

Tracking

()

RESOLVED FIXED
Bugzilla 2.16

People

(Reporter: S.Mayr2, Assigned: S.Mayr2)

Details

(Whiteboard: [fixed in 2.16.4] [does not affect trunk])

Attachments

(1 file)

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.0; de-AT; rv:1.5) Gecko/20030916
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.0; de-AT; rv:1.5) Gecko/20030916

When deleting a product, in editproducts.cgi action 'delete' the groupbit is set
to 0 for all members(profiles.groupset). The blessgroupset field is not updated.

When creating a new Product it gets this 'old' bitnumber and my users have can
turn membership bits on fo this product. That's not what I expected.

Reproducible: Always

Steps to Reproduce:
1. Create new Product A
2. Give user the right to turn bit on for other users
3. Delete Product A
4. Create new Product B
(usebuggroups has to be enabled)

Actual Results:  
Now this user can turn bit on for other users (Product B)

Expected Results:  
The blessgroupset should also be updated. Nobody except superusers can turn this
bit on.

Using Bugzilla 2.16.3
Summary: When deleting products and usebuggroups is set, blessgroupset is not handled correct → When deleting products and usebuggroups is set, blessgroupset is not updated
Version: unspecified → 2.16.3
Changing the SQL-query does it for me:

UPDATE profiles SET groupset = groupset - $bit, blessgroupset = (blessgroupset |
$bit) - $bit WHERE (groupset & $bit) AND (groupset != $::superusergroupset);

blessgroupset = (blessgroupset|$bit) - $bit
(Adding the bit to delete the correct one)

editproducts.cgi, line 688ff
Forget the case being blessed but no member :

UPDATE profiles SET groupset = groupset - $bit, blessgroupset = (blessgroupset |
$bit) - $bit WHERE ((groupset & $bit) OR (blessgroupset & $bit)) AND (groupset
!= $::superusergroupset);

Is ist possible to have superuserblessung but not being superuser ? This would
require another AND in the WHERE clause. 
groupset must be save too:

UPDATE profiles SET groupset = (groupset | $bit) - $bit, blessgroupset =
(blessgroupset |
$bit) - $bit WHERE ((groupset & $bit) OR (blessgroupset & $bit)) AND (groupset
!= $::superusergroupset);
This would be 2.16.x only - wouldn't affect 2.17.x due to the rewrite of the
groups system, most likely.

This might be worth fixing for 2.16.4, since it has potential security
implications when the bit for that product gets reused...
OS: Linux → All
Hardware: PC → All
Status: UNCONFIRMED → NEW
Ever confirmed: true
This bug did not survive into the new group system in 2.17  (I checked)

When the advisory for 2.16 goes out, we should make sure we are clear that this
would not give bless capability to anyone who had never been trusted with a
bless capability in the past.  Most sites would be very unlikely to ever see
this as a problem.  It's not like it elevates the privileges of an user who had
never been trusted.


Whiteboard: [wanted for 2.16.4] [does not affect trunk]
Target Milestone: --- → Bugzilla 2.16
Good catch, tested.
Attachment #133387 - Flags: review?(justdave)
Attachment #133387 - Flags: review?(justdave)
Attachment #133387 - Flags: review?(bugreport)
Attachment #133387 - Flags: review+
Comment on attachment 133387 [details] [diff] [review]
Update {bless,}groupset on project deletion

This reads OK.	I'd like to make sure someone tested it before it goes in. 
Release notes should include the suggestion that upgrading sites check to
ensure that residual effects of bug 157704 are not biting them.(unlikely)
r=joel
Attachment #133387 - Flags: review?(bugreport)
Assigned --> code writer.

>> I'd like to make sure someone tested it before it goes in.

Stefan and chaduv said that they already tested it. I could make an additional
test if it will help.
Assignee: justdave → S.Mayr2
Flags: approval?
Status: NEW → ASSIGNED
Yep, I tested this pretty thoroughly last night. The only question I would have 
is that it uses bit manipulations - http://bugzilla.org/developerguide.html, 
under SQL queries, says that bit arithmatic isn't universally supported. I know 
that bitwise & and | are supported in postgres, but don't know about other DBs.

As this is for 2.16 branch only, not a huge concern.
Whiteboard: [wanted for 2.16.4] [does not affect trunk] → [ready for 2.16.4] [does not affect trunk]
Yeah, 2.16 isn't headed towards supporting other DBs. :)  So that works.  We'll
hold this until the advisory is ready to go before checking in.  Thanks everyone.
Checking in editproducts.cgi;
/cvsroot/mozilla/webtools/bugzilla/editproducts.cgi,v  <--  editproducts.cgi
new revision: 1.24.2.4; previous revision: 1.24.2.3
done
Status: ASSIGNED → RESOLVED
Closed: 16 years ago
Flags: approval+
Resolution: --- → FIXED
Whiteboard: [ready for 2.16.4] [does not affect trunk] → [fixed in 2.16.4] [does not affect trunk]
Security advisory has been posted
Group: webtools-security
QA Contact: matty_is_a_geek → default-qa
You need to log in before you can comment on or make changes to this bug.