731178 - (CVE-2012-4199) [SECURITY] field-events.js.tmpl discloses product and component names that the user is not allowed to see

Status: RESOLVED FIXED

(Bugzilla :: Creating/Changing Bugs, defect)

Description

[Frédéric Buclin]

If a custom field visibility is controlled by a product or a component of a product you cannot see, their names are displayed in the JS code generated by field-events.js.tmpl:

showFieldWhen('cf_audience', 'component', [ 'very_secret_component' ]);

The UI itself has no reference to this component, but looking at the source code of the page discloses this information. I'm not sure since when this problem exists. This template exists since Bugzilla 3.4, see bug 308253, but maybe this problem has been introduced later. We would have to check.

This bug will probably be fixed by bug 695514 for trunk (and 4.2, if there is a valuable perf win).

Comment 1

[Frédéric Buclin]

I checked, and Bugzilla 3.6 and newer are all affected. Bug 695514 fixed the problem only partially. Products you cannot see are still listed in the JS code if a custom field visibility depends on them.

Comment 2

[Frédéric Buclin]

I have a patch almost ready. Taking!

Comment 3

[Frédéric Buclin]

Attachment: patch for 4.4 + trunk, v1

There is no need to do anything for showValueWhen() as it only uses IDs, not names. The patch also hides classifications for logged out users as this code is totally useless and never called (this is not strictly related to this bug, but I think it's fine for 4.4 + trunk). As a side-effect, this also fixes bug 667150.

Comment 4

[Frédéric Buclin]

Attachment: patch for 4.4 + trunk, v1.1

This makes the code a bit faster on installations with many products as I now pass the product object to can_enter_product() instead of its name only. This way, can_enter_product() doesn't need to recreate the product object again. It can uses the given one directly, meaning that this method doesn't need to call the DB again and again (the list of enterable products is cached the first time can_enter_product() is called).

Comment 5

[David Lawrence [:dkl]]

Comment on attachment 672251 [details] [diff] [review]
patch for 4.4 + trunk, v1.1

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

Looks fine and works as expected. I still do not like the fact that the private custom field (field that becomes visible when private product/component is selected) is always in the HTML whether the user can access the product/component or not. This could basically give away sensitive information from a field associated with a private product. But that is a matter for a different bug/discussion. r=dkl

Comment 6

[Frédéric Buclin]

Attachment: patch for 4.2.x, v1

Backport for 4.2. The only difference is a tiny bitrot in the first block of edit.html.tmpl. No functionality change. I still hide classifications from logged out users.

Comment 7

[David Lawrence [:dkl]]

Comment on attachment 675225 [details] [diff] [review]
patch for 4.2.x, v1

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

r=dkl

Comment 8

[Frédéric Buclin]

Attachment: patch for 4.0.x, v1

Backport for 4.0. This time, I leave all_classifications alone to have minimal changes for this branch.

Comment 9

[Frédéric Buclin]

Attachment: patch for 3.6.x, v1

Backport for 3.6. In 3.6, it's not possible to use classifications or components to restrict the visibility of fields, so this makes the patch simpler.

Comment 10

[David Lawrence [:dkl]]

Comment on attachment 675309 [details] [diff] [review]
patch for 4.0.x, v1

r=dkl

Comment 11

[David Lawrence [:dkl]]

Comment on attachment 675326 [details] [diff] [review]
patch for 3.6.x, v1

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

r=dkl

Comment 12

[Frédéric Buclin]

Committing to: bzr+ssh://[e-mail]/bugzilla/trunk/
modified template/en/default/bug/edit.html.tmpl
modified template/en/default/bug/field-events.js.tmpl
Committed revision 8466.

Committing to: bzr+ssh://[e-mail]/bugzilla/4.4/
modified template/en/default/bug/edit.html.tmpl
modified template/en/default/bug/field-events.js.tmpl
Committed revision 8451.

Committing to: bzr+ssh://[e-mail]/bugzilla/4.2/
modified template/en/default/bug/edit.html.tmpl
modified template/en/default/bug/field-events.js.tmpl
Committed revision 8165.

Committing to: bzr+ssh://[e-mail]/bugzilla/4.0/
modified template/en/default/bug/field-events.js.tmpl
modified template/en/default/bug/field.html.tmpl
modified template/en/default/bug/create/create.html.tmpl
Committed revision 7731.

Committing to: bzr+ssh://[e-mail]/bugzilla/3.6/
modified template/en/default/bug/field-events.js.tmpl
modified template/en/default/bug/field.html.tmpl
Committed revision 7305.

Comment 13

[Frédéric Buclin]

Security advisory sent. Removing the security flag.