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

Status: RESOLVED FIXED

(Bugzilla :: Administration, task)

Description

[Stefan Mayr]

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

Comment 1

[Stefan Mayr]

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

Comment 2

[Stefan Mayr]

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. 

Comment 3

[Stefan Mayr]

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);

Comment 4

[Dave Miller [:justdave]]

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...

Comment 5

[Joel Peshkin]

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.

Comment 6

[chaduv]

Attachment: Update {bless,}groupset on project deletion

Good catch, tested.

Comment 7

[Joel Peshkin]

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

Comment 8

[Vlad Dascalu]

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.

Comment 9

[chaduv]

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.

Comment 10

[Dave Miller [:justdave]]

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.

Comment 11

[Dave Miller [:justdave]]

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

Comment 12

[Dave Miller [:justdave]]

Security advisory has been posted