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 User image 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 User image 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 User image 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 User image 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 User image Daniel Veditz [:dveditz] 2010-12-21 15:33:16 PST
CVE-2010-4570
Comment 5 User image 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 User image Frédéric Buclin 2011-01-10 09:41:29 PST
I can reproduce, using the same strings as for bug 619637.
Comment 7 User image 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 User image 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 User image 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 User image Reed Loden [:reed] (use needinfo?) 2011-01-20 22:27:37 PST
Created attachment 505700 [details] [diff] [review]
patch - v2
Comment 11 User image 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 User image 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 User image 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.