Last Comment Bug 781850 - (CVE-2012-4198) [SECURITY] Do not leak the existence of groups when using User.get()
(CVE-2012-4198)
: [SECURITY] Do not leak the existence of groups when using User.get()
Status: RESOLVED FIXED
:
Product: Bugzilla
Classification: Server Software
Component: WebService (show other bugs)
: 3.7.1
: All All
: -- major (vote)
: Bugzilla 4.0
Assigned To: Frédéric Buclin
: default-qa
Mentors:
Depends on: 548198
Blocks: 805640
  Show dependency treegraph
 
Reported: 2012-08-10 09:51 PDT by Frédéric Buclin
Modified: 2012-11-14 04:30 PST (History)
3 users (show)
LpSolit: approval+
LpSolit: approval4.4+
LpSolit: blocking4.4+
LpSolit: approval4.2+
LpSolit: blocking4.2.4+
LpSolit: approval4.0+
LpSolit: blocking4.0.9+
LpSolit: testcase+
See Also:
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
patch for 4.4 + trunk, v1 (3.26 KB, patch)
2012-10-08 10:01 PDT, Frédéric Buclin
dkl: review+
Details | Diff | Review
patch for 4.2, v1 (2.33 KB, patch)
2012-10-15 05:15 PDT, Frédéric Buclin
dkl: review+
Details | Diff | Review
patch for 4.0, v1 (2.43 KB, patch)
2012-10-15 05:22 PDT, Frédéric Buclin
dkl: review+
Details | Diff | Review

Description Frédéric Buclin 2012-08-10 09:51:37 PDT
I just realized that User.get becomes way too verbose in Bugzilla 4.4. With a powerless account, I can access personal information, some of which power users shouldn't have access to either (even with admin or editusers privs):

- (confidential) groups a user belongs to, bug 548198 (Bugzilla 4.0!)
- unshared saved searches, bug 697224 (Bugzilla 4.4)

This is totally unacceptable and this information must only be disclosed if you are querying your own account, i.e. the user ID matches Bugzilla->user->id.

I know I approved bug 697224, but I didn't realize that all users could have access to this information. I clearly withdraw my approval here.

Said differently, saved searches should never be disclosed to anyone but you. From the UI, admins have no way to get this information, except by impersonating the user account, in which case the user is informed by email that someone is looking at his account. So there is no reason that someone can have access to this information using User.get, even for admins.

About groups, User.get is disclosing if a group exists or not, defeating CVE-2011-2380, see bug 653477. Bugzilla::Group->check_no_disclose() must be used instead of Bugzilla::Group->check().
Comment 1 Frédéric Buclin 2012-08-10 10:04:59 PDT
I backed out bug 697224. Only Bugzilla 4.3.2 had this code, so we are fine here. So this bug now focuses on the disclose of confidential groups only.
Comment 2 Frédéric Buclin 2012-10-08 09:59:37 PDT
(In reply to Frédéric Buclin from comment #0)
> Bugzilla::Group->check_no_disclose() must be used instead of Bugzilla::Group->check().

Actually, I simply had to fix the error ID passed to check(). check_no_disclose() must only be used for bugs.
Comment 3 Frédéric Buclin 2012-10-08 10:01:20 PDT
Created attachment 669185 [details] [diff] [review]
patch for 4.4 + trunk, v1

It's now illegal to pass a group name if you don't belong to this group (to not disclose if this group exists or not). It's still fine to pass a group ID you don't belong to, though, because its name is not disclosed.
Comment 4 David Lawrence [:dkl] 2012-10-09 15:42:16 PDT
Comment on attachment 669185 [details] [diff] [review]
patch for 4.4 + trunk, v1

Review of attachment 669185 [details] [diff] [review]:
-----------------------------------------------------------------

Looks good and works as expected. r=dkl

::: Bugzilla/WebService/User.pm
@@ +889,5 @@
>  
> +=item 804 (Invalid Group Name)
> +
> +You passed a group name which doesn't exist or which you don't belong to
> +in the C<groups> argument.

Nit: may sound better this way

You passed a group name in the C<groups> argument which either does not exist or you do not belong to it.

I am fine either way.
Comment 5 Frédéric Buclin 2012-10-09 15:57:35 PDT
I need to backport it to 4.2 and 4.0 as the patch doesn't apply cleanly to these older branches.
Comment 6 Frédéric Buclin 2012-10-15 05:15:24 PDT
Created attachment 671395 [details] [diff] [review]
patch for 4.2, v1

Patch for 4.2.x only.
Comment 7 Frédéric Buclin 2012-10-15 05:22:05 PDT
Created attachment 671396 [details] [diff] [review]
patch for 4.0, v1

Patch for 4.0.x only. The only difference compared to the patch for 4.2 is in Constants.pm.
Comment 8 David Lawrence [:dkl] 2012-10-15 21:47:59 PDT
Comment on attachment 671395 [details] [diff] [review]
patch for 4.2, v1

Review of attachment 671395 [details] [diff] [review]:
-----------------------------------------------------------------

r=dkl
Comment 9 David Lawrence [:dkl] 2012-10-15 21:48:21 PDT
Comment on attachment 671396 [details] [diff] [review]
patch for 4.0, v1

Review of attachment 671396 [details] [diff] [review]:
-----------------------------------------------------------------

r=dkl
Comment 10 Frédéric Buclin 2012-10-26 10:39:15 PDT
No need for another CVE number for this bug (I think), it's another form of CVE-2011-2380.
Comment 11 Daniel Veditz [:dveditz] 2012-10-29 10:38:51 PDT
You can't re-use year-old CVEs that way, use CVE-2012-4198 instead.
Comment 12 Frédéric Buclin 2012-11-13 09:38:08 PST
Committing to: bzr+ssh://lpsolit%40gmail.com@bzr.mozilla.org/bugzilla/trunk/
modified Bugzilla/WebService/Constants.pm
modified Bugzilla/WebService/User.pm
Committed revision 8468.

Committing to: bzr+ssh://lpsolit%40gmail.com@bzr.mozilla.org/bugzilla/4.4/
modified Bugzilla/WebService/Constants.pm
modified Bugzilla/WebService/User.pm
Committed revision 8453.

Committing to: bzr+ssh://lpsolit%40gmail.com@bzr.mozilla.org/bugzilla/4.2/
modified Bugzilla/WebService/Constants.pm
modified Bugzilla/WebService/User.pm
Committed revision 8167.

Committing to: bzr+ssh://lpsolit%40gmail.com@bzr.mozilla.org/bugzilla/4.0/
modified Bugzilla/WebService/Constants.pm
modified Bugzilla/WebService/User.pm
Committed revision 7733.
Comment 13 Frédéric Buclin 2012-11-13 13:32:53 PST
Committing to: bzr+ssh://lpsolit%40gmail.com@bzr.mozilla.org/bugzilla/qa/4.4/
modified t/webservice_user_get.t
Committed revision 235.

Committing to: bzr+ssh://lpsolit%40gmail.com@bzr.mozilla.org/bugzilla/qa/4.2/
modified t/webservice_user_get.t
Committed revision 227.

Committing to: bzr+ssh://lpsolit%40gmail.com@bzr.mozilla.org/bugzilla/qa/4.0/
modified t/webservice_user_get.t
Committed revision 208.
Comment 14 Frédéric Buclin 2012-11-14 04:30:19 PST
Security advisory sent. Removing the security flag.

Note You need to log in before you can comment on or make changes to this bug.