Closed Bug 653341 Opened 10 years ago Closed 10 years ago

Bug.create() fails to error out if an invalid group is passed

Categories

(Bugzilla :: WebService, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
Bugzilla 4.0

People

(Reporter: justdave, Assigned: LpSolit)

Details

Attachments

(1 file, 3 obsolete files)

Like the summary says, Bug.create() fails to error out if an invalid group is passed.  As far as I can tell from the source, this was intentional, so as to not disclose group names.  However, it has the unfortunate side effect of causing automated tools which submit confidential bugs to suddenly start filing them in public when a group name changes (or in this case when the API changed to require group names instead of IDs), which caused an information disclosure on mozilla.org this morning.
I suppose this is my "I told you so" moment. :-)

I don't think we need to keep this confidential, but I do agree that it's a high-priority issue that needs to be fixed.
Group: bugzilla-security
Target Milestone: --- → Bugzilla 4.0
Flags: blocking4.0.2+
And yeah, I guess I can yell at the webdev guys for using an "undocumented" API (because groups weren't documented at all in 3.6, but it was there in the source), but the name change thing could still bite someone in the future.
Target Milestone: Bugzilla 4.0 → ---
Target Milestone: --- → Bugzilla 4.0
We should reject the bug submission, and the error message should be similar to what we display in the web browser, i.e. that "either the group Foo doesn't exist, or you are not allowed to restrict bugs to this group in this product". This way, we don't disclose the existence of the group.

Decreasing severity as Bug.create() works as the doc says.
Severity: critical → normal
OS: Mac OS X → All
Hardware: x86 → All
Attached patch patch, v1 (obsolete) — Splinter Review
Assignee: webservice → LpSolit
Status: NEW → ASSIGNED
Attachment #528902 - Flags: review?(mkanat)
Attached patch patch, v1.1 (obsolete) — Splinter Review
The comment is now obsolete.
Attachment #528902 - Attachment is obsolete: true
Attachment #528902 - Flags: review?(mkanat)
Attachment #528905 - Flags: review?(mkanat)
Comment on attachment 528905 [details] [diff] [review]
patch, v1.1

Review of attachment 528905 [details] [diff] [review]:

Awesome, looks great. :-)
Attachment #528905 - Flags: review?(mkanat) → review+
We should also make sure that update() behaves similarly.
Flags: approval4.0+
Flags: approval+
Attached patch patch, v1.2 (obsolete) — Splinter Review
We also have to update the API documentation as we no longer silently ignore illegal group names.
Attachment #528905 - Attachment is obsolete: true
Attachment #529074 - Flags: review?(mkanat)
Attached patch patch, v1.3Splinter Review
Still another place to fix, to mention that Bug.create can now return error 120.
Attachment #529074 - Attachment is obsolete: true
Attachment #529074 - Flags: review?(mkanat)
Attachment #529075 - Flags: review?(mkanat)
(In reply to comment #1)
> I suppose this is my "I told you so" moment. :-)

Max: I thought you were the one arguing for the "don't reveal group names" behaviour? Or am I confused?

Gerv
(In reply to comment #10)
> Max: I thought you were the one arguing for the "don't reveal group names"
> behaviour? Or am I confused?

  No, I heavily believed that Bugzilla should be as explicit and loud as possible about invalid groups.
Comment on attachment 529075 [details] [diff] [review]
patch, v1.3

Review of attachment 529075 [details] [diff] [review]:

Looks great. :-)
Attachment #529075 - Flags: review?(mkanat) → review+
Committing to: bzr+ssh://lpsolit%40gmail.com@bzr.mozilla.org/bugzilla/trunk/
modified Bugzilla/Bug.pm
modified Bugzilla/WebService/Bug.pm
modified Bugzilla/WebService/Constants.pm
modified template/en/default/global/user-error.html.tmpl
Committed revision 7808.

Committing to: bzr+ssh://lpsolit%40gmail.com@bzr.mozilla.org/bugzilla/4.0/
modified Bugzilla/Bug.pm
modified Bugzilla/WebService/Bug.pm
modified Bugzilla/WebService/Constants.pm
modified template/en/default/global/user-error.html.tmpl
Committed revision 7593.
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Committing to: bzr+ssh://lpsolit%40gmail.com@bzr.mozilla.org/bugzilla/qa/4.2/
modified t/webservice_bug_create.t
Committed revision 212.

Committing to: bzr+ssh://lpsolit%40gmail.com@bzr.mozilla.org/bugzilla/qa/4.0/
modified t/webservice_bug_create.t
Committed revision 201.
Flags: testcase+
You need to log in before you can comment on or make changes to this bug.