The default bug view has changed. See this FAQ.

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

RESOLVED FIXED in Bugzilla 4.0

Status

()

Bugzilla
WebService
RESOLVED FIXED
6 years ago
6 years ago

People

(Reporter: justdave, Assigned: Frédéric Buclin)

Tracking

Bugzilla 4.0
Bug Flags:
approval +
approval4.0 +
blocking4.0.2 +
testcase +

Details

Attachments

(1 attachment, 3 obsolete attachments)

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.

Comment 1

6 years ago
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

Updated

6 years ago
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
(Assignee)

Comment 3

6 years ago
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
(Assignee)

Comment 4

6 years ago
Created attachment 528902 [details] [diff] [review]
patch, v1
Assignee: webservice → LpSolit
Status: NEW → ASSIGNED
Attachment #528902 - Flags: review?(mkanat)
(Assignee)

Comment 5

6 years ago
Created attachment 528905 [details] [diff] [review]
patch, v1.1

The comment is now obsolete.
Attachment #528902 - Attachment is obsolete: true
Attachment #528902 - Flags: review?(mkanat)
Attachment #528905 - Flags: review?(mkanat)

Comment 6

6 years ago
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+

Comment 7

6 years ago
We should also make sure that update() behaves similarly.
Flags: approval4.0+
Flags: approval+
(Assignee)

Comment 8

6 years ago
Created attachment 529074 [details] [diff] [review]
patch, v1.2

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)
(Assignee)

Comment 9

6 years ago
Created attachment 529075 [details] [diff] [review]
patch, v1.3

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

Comment 11

6 years ago
(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 12

6 years ago
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+
(Assignee)

Comment 13

6 years ago
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
Last Resolved: 6 years ago
Resolution: --- → FIXED
(Assignee)

Comment 14

6 years ago
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.