Closed Bug 781850 (CVE-2012-4198) Opened 8 years ago Closed 8 years ago

[SECURITY] Do not leak the existence of groups when using User.get()

Categories

(Bugzilla :: WebService, defect, major)

3.7.1
defect
Not set
major

Tracking

()

RESOLVED FIXED
Bugzilla 4.0

People

(Reporter: LpSolit, Assigned: LpSolit)

References

Details

Attachments

(3 files)

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().
Flags: blocking4.4+
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.
Severity: critical → major
(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.
Assignee: webservice → LpSolit
Status: NEW → ASSIGNED
Flags: blocking4.2.4+
Flags: blocking4.0.9+
Summary: Do not leak personal information to all users → [SECURITY] Do not leak the existence of groups when using User.get()
Target Milestone: Bugzilla 4.4 → Bugzilla 4.0
Version: 4.3.2 → 3.7.1
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.
Attachment #669185 - Flags: review?(dkl)
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.
Attachment #669185 - Flags: review?(dkl) → review+
Attachment #669185 - Attachment description: patch, v1 → patch for 4.4 + trunk, v1
I need to backport it to 4.2 and 4.0 as the patch doesn't apply cleanly to these older branches.
Flags: approval?
Flags: approval4.4?
Patch for 4.2.x only.
Attachment #671395 - Flags: review?(dkl)
Patch for 4.0.x only. The only difference compared to the patch for 4.2 is in Constants.pm.
Attachment #671396 - Flags: review?(dkl)
Comment on attachment 671395 [details] [diff] [review]
patch for 4.2, v1

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

r=dkl
Attachment #671395 - Flags: review?(dkl) → review+
Comment on attachment 671396 [details] [diff] [review]
patch for 4.0, v1

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

r=dkl
Attachment #671396 - Flags: review?(dkl) → review+
Flags: approval4.2?
Flags: approval4.0?
Blocks: 805640
No need for another CVE number for this bug (I think), it's another form of CVE-2011-2380.
Alias: CVE-2011-2380-p2
You can't re-use year-old CVEs that way, use CVE-2012-4198 instead.
Alias: CVE-2011-2380-p2 → CVE-2012-4198
No longer depends on: 697224
Flags: approval?
Flags: approval4.4?
Flags: approval4.4+
Flags: approval4.2?
Flags: approval4.2+
Flags: approval4.0?
Flags: approval4.0+
Flags: approval+
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.
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
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.
Flags: testcase+
Security advisory sent. Removing the security flag.
Group: bugzilla-security
You need to log in before you can comment on or make changes to this bug.