Closed
Bug 772998
Opened 12 years ago
Closed 12 years ago
Persistent (stored) XSS in profile allowing client side code to be executed via private Messages.
Categories
(support.mozilla.org :: General, defect)
support.mozilla.org
General
Tracking
(Not tracked)
RESOLVED
FIXED
People
(Reporter: cocking70, Assigned: jsocol)
Details
(Keywords: sec-critical, wsec-xss)
User Agent: Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/535.11 (KHTML, like Gecko) Chrome/17.0.963.79 Safari/535.11 Steps to reproduce: After creating an account and going to the profile editor (https://support.mozilla.org/en-US/users/edit) I changed "Display name:" to: "/><script>alert(document.cookie);</script> I then tried to send a private message (https://support.mozilla.org/en-US/message/new) to this account, the client side code was be executed. Actual results: An alert box was created showing cookie information. This persistent XSS could be used to steal sessions from other users when they attempt to private message me without any user interaction. Proof of concept: (using Firefox) In the "To" box (found here: https://support.mozilla.org/en-US/message/new) if you type my account name "testaudit" (without the quotes) you will see an alert box with your cookie information displayed. Expected results: Username should appear in TO box allowing private message to be sent.
I would also like to include that if i created multiple accounts and used the following display names: a"/><script>blahblah</script> b"/><script>blahblah</script> ... z"/><script>blahblah</script> As the To function searches for display names as well as user names & the auto complete feature is enabled, it would execute my client side script after the first character is entered. This means the session of every single person who sends a PM would be stolen without any user interaction. Hopefully you agree, this bug could have been devastating.
Updated•12 years ago
|
Group: core-security → websites-security
Updated•12 years ago
|
Keywords: sec-critical,
wsec-xss
Whiteboard: ws:critical
Assignee | ||
Comment 3•12 years ago
|
||
Ricky, let's tackle this ASAP and get a fix out today if possible. Looks like we're using string concatenation[1] when we should be assigning innerHTML (or adding a new function that escapes and interpolates). [1] https://github.com/mozilla/kitsune/blob/master/media/js/users.autocomplete.js#L40
Status: UNCONFIRMED → NEW
Ever confirmed: true
Assignee | ||
Updated•12 years ago
|
Assignee: nobody → james
Assignee | ||
Comment 4•12 years ago
|
||
https://github.com/mozilla/kitsune/pull/717
Comment 5•12 years ago
|
||
Landed the commit to master. Will push to stage then prod shortly. https://github.com/mozilla/kitsune/commit/9416717ed28cf553020b54976d62a8e61b5e0ff5
Comment 6•12 years ago
|
||
Just landed in prod.
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Assignee | ||
Comment 7•12 years ago
|
||
Once this is verified let's remove it from the websites-security group. mfuller can you help us verify this?
Comment 8•12 years ago
|
||
Hi, I can verify this, but the page the OP listed (https://support.mozilla.org/en-US/message/new) is returning a "not found" for me. Also, is the "display name" written to a database? If so, the correct fix for this cannot be JavaScript / blacklist-based. For example, a user can craft a JavaScript injection that does not include the "unsafe" characters (&, <, >, etc). If the name is retrieved from a database, it must have a server-side escaping function before it can be used in JS or on the page. Let me know and I can look at this further Thanks! Matt
Go here: https://support.mozilla.org/en-US/messages then click: "New Message" this will take you to /messages/new
Assignee | ||
Comment 10•12 years ago
|
||
(In reply to Matt Fuller :mfuller from comment #8) > Also, is the "display name" written to a database? If so, the correct fix > for this cannot be JavaScript / blacklist-based. For example, a user can > craft a JavaScript injection that does not include the "unsafe" characters > (&, <, >, etc). Yes, of course it's written to a database. The string is never executed as JavaScript, it's always treated as a string literal. The issue here was that it was being concatenated as HTML, not as JS. Escaping all HTML special characters (which is literally all that the Python version of this function does) mitigates the HTML-injection vector. There are certainly injections that don't use any HTML special chars, (stuff like [!{}]+[+[]])[+[]]) but those are meaningless in HTML. > If the name is retrieved from a database, it must have a server-side > escaping function before it can be used in JS or on the page. It is escaped on the server insofar as it is guaranteed to be valid JSON, which means the string on the server side turns into a JS literal string before being sent to the browser. It's entirely possible there's a vector I don't know about, so if you could explain this further, maybe give an example, that would be great. We're talking about a JSON-encoded JS string that is never evaled as JS. It was inserted into HTML, and now it's inserted via a function that escapes all HTML special chars.
Assignee | ||
Comment 11•12 years ago
|
||
(In reply to cocking70 from comment #9) > Go here: https://support.mozilla.org/en-US/messages > then click: "New Message" > > this will take you to /messages/new You do need to be logged in, of course.
Comment 12•12 years ago
|
||
:james - thanks for the clarification. I was asking about whether it was escaped because there may be other locations where the name is displayed as raw HTML (a classic XSS). I understand what you're referring to with the function, that is sufficient. My concern was if the following event occurred: - User inserts malicious payload as display name - Display name is included as part of a static HTML generated page (reflected) - Code is executed. That doesn't appear to be the case, as you've described. Thanks for explaining, I've verified this fix. Matt
Status: RESOLVED → VERIFIED
Assignee | ||
Comment 13•12 years ago
|
||
(In reply to Matt Fuller :mfuller from comment #12) > :james - thanks for the clarification. I was asking about whether it was > escaped because there may be other locations where the name is displayed as > raw HTML (a classic XSS). Output is escaped at the template level in cases like that. Most user-generated content is auto-escaped by our template layer. Interpolation in general marks a string as "unsafe" to the template layer. Some (e.g. that which must be wrapped non-trivially in HTML, usually for L10n reasons, like 'welcome, <a href="/profile/url">username</a>') uses the |fe filter of jingo[1]. This escapes user content before interpolating and marking the whole thing as safe. So, user data is either escaped outright or escaped before being interpolated (like this JS function now does) or passed through Bleach[2]. Not claiming that we're perfect, but we're close, and those are our mitigations for reflected XSS. [1] https://github.com/jbalogh/jingo/blob/master/jingo/helpers.py#L30 [2] https://github.com/jsocol/bleach The code here operates exactly like the |fe filter.
Status: VERIFIED → RESOLVED
Closed: 12 years ago → 12 years ago
Assignee | ||
Comment 14•12 years ago
|
||
(In reply to Ricky Rosario [:rrosario, :r1cky] from comment #6) > Just landed in prod. (In reply to Matt Fuller :mfuller from comment #12) > Thanks for explaining, I've verified this fix. Can we go ahead and remove this from the websites-security group? I'd like to use it as an example.
Comment 15•12 years ago
|
||
Thanks everyone - I've removed it from websites-security.
Group: websites-security
Updated•11 years ago
|
Flags: sec-bounty+
Comment 18•8 years ago
|
||
The reporter would prefer to be credited in the HoF as "Daniel Cocking" (per mail to security@)
You need to log in
before you can comment on or make changes to this bug.
Description
•