Bug 619648 (CVE-2010-4570)

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

RESOLVED FIXED in Bugzilla 4.0

Status

()

Bugzilla
User Interface
--
major
RESOLVED FIXED
7 years ago
4 years ago

People

(Reporter: reed, Assigned: reed)

Tracking

(Blocks: 1 bug)

3.7.1
Bugzilla 4.0
Dependency tree / graph
Bug Flags:
approval +
approval4.0 +
blocking4.0 +

Details

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

Attachments

(1 attachment, 1 obsolete attachment)

759 bytes, patch
Max Kanat-Alexander
: review+
Details | Diff | Splinter Review
(Assignee)

Description

7 years ago
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.
Flags: blocking4.0?
Attachment #498075 - Flags: review?(mkanat)
(Assignee)

Updated

7 years ago
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
(Assignee)

Comment 3

6 years ago
(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.

Updated

6 years ago
Blocks: 620540
(Assignee)

Updated

6 years ago
Whiteboard: [infrasec:xss][ws:high] → [infrasec:xss][ws:critical?]
CVE-2010-4570
Alias: CVE-2010-4570

Updated

6 years ago
Attachment #498075 - Flags: review?(guy.pyrzak)

Updated

6 years ago
Flags: blocking4.0? → blocking4.0+

Comment 5

6 years ago
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

6 years ago
I can reproduce, using the same strings as for bug 619637.

Comment 7

6 years ago
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 8

6 years ago
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-
(Assignee)

Comment 9

6 years ago
(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.
(Assignee)

Comment 10

6 years ago
Created attachment 505700 [details] [diff] [review]
patch - v2
Attachment #498075 - Attachment is obsolete: true
Attachment #505700 - Flags: review?(mkanat)

Comment 11

6 years ago
Comment on attachment 505700 [details] [diff] [review]
patch - v2

Looks right to me.
Attachment #505700 - Flags: review?(mkanat) → review+

Updated

6 years ago
Flags: approval?
Flags: approval4.0?

Updated

6 years ago
Version: 3.7.3 → 3.7.1

Updated

6 years ago
Flags: approval?
Flags: approval4.0?
Flags: approval4.0+
Flags: approval+
(Assignee)

Comment 13

6 years ago
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
Last Resolved: 6 years ago
Resolution: --- → FIXED
(Assignee)

Updated

6 years ago
Whiteboard: [infrasec:xss][ws:critical?] → [infrasec:xss][ws:critical]

Comment 14

6 years ago
Security advisory sent. Removing the security flag.
Group: bugzilla-security

Updated

4 years ago
Blocks: 835424
You need to log in before you can comment on or make changes to this bug.