Closed Bug 619648 (CVE-2010-4570) Opened 13 years ago Closed 13 years ago

[SECURITY] XSS via summary in "possible duplicates" table due to lack of encoding by YUI

Categories

(Bugzilla :: User Interface, defect)

3.7.1
defect
Not set
major

Tracking

()

RESOLVED FIXED
Bugzilla 4.0

People

(Reporter: reed, Assigned: reed)

References

Details

(Whiteboard: [infrasec:xss][ws:critical])

Attachments

(1 file, 1 obsolete file)

Attached patch patch - v1 (obsolete) — Splinter Review
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.
Flags: blocking4.0?
Attachment #498075 - Flags: review?(mkanat)
Summary: [SECURITY] XSS via short_desc in possible duplicate detection table → [SECURITY] XSS via summary in "possible duplicates" table due to lack of encoding by YUI
Isn't the "FILTER js" supposed to prevent script injection? If it doesn't, why do we have it?

Gerv
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
(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.
Blocks: 620540
Whiteboard: [infrasec:xss][ws:high] → [infrasec:xss][ws:critical?]
CVE-2010-4570
Alias: CVE-2010-4570
Attachment #498075 - Flags: review?(guy.pyrzak)
Flags: blocking4.0? → blocking4.0+
I do need somebody to confirm that this is actually valid--I don't see any comments in the bug saying that it is.
I can reproduce, using the same strings as for bug 619637.
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.
Attachment #498075 - Flags: review?(guy.pyrzak)
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"
Attachment #498075 - Flags: review?(mkanat) → review-
(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.
Attached patch patch - v2Splinter Review
Attachment #498075 - Attachment is obsolete: true
Attachment #505700 - Flags: review?(mkanat)
Comment on attachment 505700 [details] [diff] [review]
patch - v2

Looks right to me.
Attachment #505700 - Flags: review?(mkanat) → review+
Flags: approval?
Flags: approval4.0?
Version: 3.7.3 → 3.7.1
Flags: approval?
Flags: approval4.0?
Flags: approval4.0+
Flags: approval+
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.
Status: ASSIGNED → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
Whiteboard: [infrasec:xss][ws:critical?] → [infrasec:xss][ws:critical]
Security advisory sent. Removing the security flag.
Group: bugzilla-security
You need to log in before you can comment on or make changes to this bug.