Last Comment Bug 790296 - (CVE-2012-4189) [SECURITY] Field values are not escaped correctly in tabular reports
(CVE-2012-4189)
: [SECURITY] Field values are not escaped correctly in tabular reports
Status: RESOLVED FIXED
: regression, sec-critical, wsec-xss
Product: Bugzilla
Classification: Server Software
Component: Reporting/Charting (show other bugs)
: 4.1.1
: All All
: -- major (vote)
: Bugzilla 4.2
Assigned To: Frédéric Buclin
: default-qa
Mentors:
https://landfill.bugzilla.org/bugzill...
Depends on: 142394
Blocks: 835424 805640
  Show dependency treegraph
 
Reported: 2012-09-11 09:54 PDT by Mateusz Goik
Modified: 2014-07-24 13:43 PDT (History)
8 users (show)
LpSolit: approval+
LpSolit: approval4.4+
LpSolit: blocking4.4+
LpSolit: approval4.2+
LpSolit: blocking4.2.4+
rforbes: sec‑bounty+
See Also:
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
patch, v1 (550 bytes, patch)
2012-09-11 12:20 PDT, Frédéric Buclin
no flags Details | Diff | Splinter Review
patch, v2 (1.66 KB, patch)
2012-09-12 07:28 PDT, Frédéric Buclin
dkl: review+
Details | Diff | Splinter Review

Description Mateusz Goik 2012-09-11 09:54:11 PDT
PoC:

http://localhost/cgi-bin/bug/editversions.cgi?action=add&product=TestProduct ->
Version: "><script>alert(1);</script>

Add new bug to "TestProduct" with version "><script>alert(1);</script>

http://localhost/cgi-bin/bug/query.cgi?format=report-table -> 
Horizontal Axis: Version
should be the results: Version: "><script>alert(1);</script>
-> Generate Report

http://localhost/cgi-bin/bug/report.cgi?x_axis_field=version&y_axis_field=&z_axis_field=&query_format=report-table&short_desc_type=allwordssubstr&short_desc=&resolution=---&longdesc_type=allwordssubstr&longdesc=&bug_file_loc_type=allwordssubstr&bug_file_loc=&keywords_type=allwords&keywords=&deadlinefrom=&deadlineto=&bug_id=&bug_id_type=anyexact&version=%22%3E%3Cscript%3Ealert%281%29%3B%3C%2Fscript%3E&emailassigned_to1=1&emailtype1=substring&email1=&emailassigned_to2=1&emailreporter2=1&emailcc2=1&emailtype2=substring&email2=&emaillongdesc3=1&emailtype3=substring&email3=&chfieldvalue=&chfieldfrom=&chfieldto=Now&j_top=AND&f1=noop&o1=noop&v1=&format=table&action=wrap

Result:

+ oColumn.field + "&amp;version="><script>alert(1);</script>'>"
 elLiner.innerHTML = "<a href='buglist.cgi?action=wrap&amp;resolution=---&amp;version="><script>alert(1);</script>'>"
     <a href="buglist.cgi?action=wrap&amp;resolution=---&amp;version="><script>alert(1);</script>">5</a>
<a href="buglist.cgi?action=wrap&amp;resolution=---&amp;=%20&amp;version="><script>alert(1);</script>">5</a>
Comment 1 Frédéric Buclin 2012-09-11 11:22:04 PDT
Confirmed. Bugzilla 4.0 and older are not affected as we use YUI for tabular reports since 4.2 only, see bug 142394. If JavaScript is disabled, the problem goes away, so I suspect < and > are not escaped properly somewhere in our JS code, which is surprising as < and > are supposed to be escaped by |FILTER js|, see bug 503980 and bug 670169.
Comment 2 Frédéric Buclin 2012-09-11 12:20:46 PDT
Created attachment 660184 [details] [diff] [review]
patch, v1
Comment 3 Reed Loden [:reed] (use needinfo?) 2012-09-11 12:36:39 PDT
Comment on attachment 660184 [details] [diff] [review]
patch, v1

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

Are you sure this fixes all four different spots? Seems like it would only fix 2-3 of the 4.
Comment 4 Frédéric Buclin 2012-09-11 13:35:37 PDT
(In reply to Reed Loden [:reed] from comment #3)
> Are you sure this fixes all four different spots? Seems like it would only
> fix 2-3 of the 4.

This fixes all links when JS is disabled, prevent XSS with JS disabled/enabled and links with JS enabled are only broken when the values have semicolons in them. I will check what I can do for them later (not a regression due to my patch).
Comment 5 Frédéric Buclin 2012-09-12 07:28:25 PDT
Created attachment 660428 [details] [diff] [review]
patch, v2

Correctly escape column headers.

I suspect that YUI reverts some escaping internally, because replacing oColumn.field by YAHOO.lang.escapeHTML(oColumn.field) has no effect. But that's another bug.
Comment 7 Al Billings [:abillings] 2012-09-24 15:26:29 PDT
Frederic, can you give feedback on this as far as a security rating? Is this a simple XSS issue?
Comment 8 Frédéric Buclin 2012-09-24 17:01:46 PDT
(In reply to Al Billings [:abillings] from comment #7)
> Frederic, can you give feedback on this as far as a security rating? Is this
> a simple XSS issue?

Unfortunately, it's very easy to trigger XSS, see the link in the URL field. There is no need for the value to exist in the DB.
Comment 9 David Lawrence [:dkl] 2012-10-03 12:20:03 PDT
Comment on attachment 660428 [details] [diff] [review]
patch, v2

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

r=dkl
Comment 10 Daniel Veditz [:dveditz] 2012-10-04 11:10:03 PDT
Call this one CVE-2012-4189
Comment 11 Frédéric Buclin 2012-11-13 09:56:49 PST
Committing to: bzr+ssh://lpsolit%40gmail.com@bzr.mozilla.org/bugzilla/trunk/
modified report.cgi
modified template/en/default/reports/report-table.html.tmpl
Committed revision 8470.

Committing to: bzr+ssh://lpsolit%40gmail.com@bzr.mozilla.org/bugzilla/4.4/
modified report.cgi
modified template/en/default/reports/report-table.html.tmpl
Committed revision 8455.

Committing to: bzr+ssh://lpsolit%40gmail.com@bzr.mozilla.org/bugzilla/4.2/
modified report.cgi
modified template/en/default/reports/report-table.html.tmpl
Committed revision 8169.
Comment 12 Frédéric Buclin 2012-11-14 04:30:23 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.