Last Comment Bug 619648 - (CVE-2010-4570) [SECURITY] XSS via summary in "possible duplicates" table due to lack of encoding by YUI
(CVE-2010-4570)
: [SECURITY] XSS via summary in "possible duplicates" table due to lack of enco...
Status: RESOLVED FIXED
[infrasec:xss][ws:critical]
:
Product: Bugzilla
Classification: Server Software
Component: User Interface (show other bugs)
: 3.7.1
: All All
: -- major (vote)
: Bugzilla 4.0
Assigned To: Reed Loden [:reed] (use needinfo?)
: default-qa
Mentors:
Depends on:
Blocks: 835424 620540
  Show dependency treegraph
 
Reported: 2010-12-16 01:49 PST by Reed Loden [:reed] (use needinfo?)
Modified: 2013-01-28 10:07 PST (History)
5 users (show)
LpSolit: approval+
LpSolit: approval4.0+
LpSolit: blocking4.0+
See Also:
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
patch - v1 (786 bytes, patch)
2010-12-16 01:49 PST, Reed Loden [:reed] (use needinfo?)
mkanat: review-
Details | Diff | Splinter Review
patch - v2 (759 bytes, patch)
2011-01-20 22:27 PST, Reed Loden [:reed] (use needinfo?)
mkanat: review+
Details | Diff | Splinter Review

Description Reed Loden [:reed] (use needinfo?) 2010-12-16 01:49:14 PST
Created attachment 498075 [details] [diff] [review]
patch - v1

I haven't confirmed that this actually is valid, but from what crazy YUI stuff I've seen in the last hour, I suspect it probably is.
Comment 1 Gervase Markham [:gerv] 2010-12-16 09:19:52 PST
Isn't the "FILTER js" supposed to prevent script injection? If it doesn't, why do we have it?

Gerv
Comment 2 Gervase Markham [:gerv] 2010-12-16 09:21:10 PST
Oh no, right, that just makes it encoded in the JS structure. When used, it could of course have < and > in it.

Further attempt at a useful comment: if we set the value of the cell from JS using foo.value, this won't be a problem. If we set it using foo.innerHTML, it will.

Gerv
Comment 3 Reed Loden [:reed] (use needinfo?) 2010-12-16 11:10:02 PST
(In reply to comment #1)
> Isn't the "FILTER js" supposed to prevent script injection? If it doesn't, why
> do we have it?

The js filter is only for the label (the header) for that column. It doesn't have anything to do with the actual summary text at all. That comes from js/bug.js via calls to jsonrpc.cgi.
Comment 4 Daniel Veditz [:dveditz] 2010-12-21 15:33:16 PST
CVE-2010-4570
Comment 5 Max Kanat-Alexander 2011-01-10 01:28:07 PST
I do need somebody to confirm that this is actually valid--I don't see any comments in the bug saying that it is.
Comment 6 Frédéric Buclin 2011-01-10 09:41:29 PST
I can reproduce, using the same strings as for bug 619637.
Comment 7 Guy Pyrzak 2011-01-12 15:10:09 PST
Comment on attachment 498075 [details] [diff] [review]
patch - v1

this does apply the formatText filter, which is a good thing to do, it formats out the <, > and &, but i'm not 100% certain that this would stop all XXS attacks, because i don't know much about the various ways to do an XXS attack.
Comment 8 Max Kanat-Alexander 2011-01-20 16:58:12 PST
Comment on attachment 498075 [details] [diff] [review]
patch - v1

>=== modified file 'template/en/default/bug/create/create.html.tmpl'
>+                label: "[% field_descs.short_desc FILTER js %]",
>+                formatter: YAHOO.widget.DataTable.formatText },

  For the sake of forwards-compatibility, that should be:

  formatter: "text"
Comment 9 Reed Loden [:reed] (use needinfo?) 2011-01-20 22:25:19 PST
(In reply to comment #8)
> Comment on attachment 498075 [details] [diff] [review]
> patch - v1
> 
> >=== modified file 'template/en/default/bug/create/create.html.tmpl'
> >+                label: "[% field_descs.short_desc FILTER js %]",
> >+                formatter: YAHOO.widget.DataTable.formatText },
> 
>   For the sake of forwards-compatibility, that should be:
> 
>   formatter: "text"

YUI docs call the aliases such as the above "convenience[s]", so I highly doubt that we'd run into a forwards-compat issue, but I'm happy to change it.
Comment 10 Reed Loden [:reed] (use needinfo?) 2011-01-20 22:27:37 PST
Created attachment 505700 [details] [diff] [review]
patch - v2
Comment 11 Max Kanat-Alexander 2011-01-20 22:28:11 PST
Comment on attachment 505700 [details] [diff] [review]
patch - v2

Looks right to me.
Comment 13 Reed Loden [:reed] (use needinfo?) 2011-01-24 10:09:41 PST
Committing to: bzr+ssh://bzr.mozilla.org/bugzilla/trunk/
modified template/en/default/bug/create/create.html.tmpl
Committed revision 7672.
Committing to: bzr+ssh://bzr.mozilla.org/bugzilla/4.0/
modified template/en/default/bug/create/create.html.tmpl
Committed revision 7529.
Comment 14 Frédéric Buclin 2011-01-24 17:20:04 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.