766924 - XSS vulnerability of input.mozilla.org

Status: VERIFIED FIXED

(Websites :: other.mozilla.org, defect)

Description

[mala]

User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_7_4) AppleWebKit/537.1 (KHTML, like Gecko) Chrome/21.0.1180.0 Safari/537.1

Steps to reproduce:

http://input.mozilla.org/en-US/feedback#<img src=/ onerror=alert(1)>



Actual results:

feedback.js has XSS vulnerability with jQuery, 
$('#' + hash) in here

https://github.com/fwenzel/reporter/blob/master/media/js/feedback.js#L81

see also http://bugs.jquery.com/ticket/9521

Comment 1

[Matt Fuller :mfuller]

Definitely a valid XSS - marked as New and I'll look for a developer.

Comment 2

[Matt Fuller :mfuller]

:james, can you take a look at this? Thanks!

Comment 3

[James Socol [:jsocol, :james]]

While I take a look, can someone from AppSec fill in the relevant whiteboard/keyword info like severity. And I think this is ws-reflected-xss but I don't remember all the new codes/keywords.

Comment 4

[James Socol [:jsocol, :james]]

Also, Matt, is this the line we're talking about?

https://github.com/mozilla/input.mozilla.org/blob/master/media/js/feedback.js#L83

Comment 5

[Matt Fuller :mfuller]

I am adding the whiteboard/keywords, :curtisk, can you fix them if they're incorrect. Also, :james, let me do some testing on that page and get back to you - I believe that is the correct line (the one the reporter posted) but I'll get back to you on that shortly.

Comment 6

[James Socol [:jsocol, :james]]

Nigel, if you're familiar with feedback.js, and want to take this, you'd be my hero, or else if you can just help me review a patch in a little bit.

Comment 7

[Nigel Babu [:nigelb]]

I'm not familiar with feedback.js, I'll help review the code.

Comment 8

[Matt Fuller :mfuller]

That does seem to be the line causing issues. For more information about the bug, here are some helpful links:

The JQuery Bug: http://bugs.jquery.com/ticket/9521

A Demo with Description and fix: http://ma.la/jquery_xss/

Thanks guys!

Comment 9

[mala]

Hi, there are 3 way to fix this issue.

1. update jQuery
2. $(document).find("#" + hash)
3. hash = hash.replace(/<|>/g, "")

1 will solve other potential problems. It is the best solution if the problem of compatibility does not occur.

Comment 10

[James Socol [:jsocol, :james]]

https://github.com/mozilla/input.mozilla.org/pull/49

I took a 4th option, whitelist valid values for this. I could add #2 as well. We don't really have the dev cycles right now to try to upgrade jQuery, and #3 is a blacklist, which makes me nervous (someone could find a way through that with something else).

Comment 11

[James Socol [:jsocol, :james]]

This pull request was r=nigelb and merged, also cherry-picked to the prod branch.

CCing mbrandt to help verify. (input-dev.allizom.org seems to have picked up the new code OK).

Comment 12

[Matt Brandt [:mbrandt]]

Attachment: qa - verified

QA verified on dev - followed steps to reproduce in comment 0

http://input-dev.allizom.org/en-US/feedback#%3Cimg%20src=/%20onerror=alert%281%29%3E

Comment 13

[Matt Fuller :mfuller]

Wow, thanks for the fast fix guys!

Comment 14

[Matt Brandt [:mbrandt]]

The patch just landed on prod (bug 767108) and has been verified. Thank you to all who helped identify, patch, and get this landed on prod.

Comment 15

[Matt Brandt [:mbrandt]]

Reopening: it appears that the push reverted somehow and the vulnerability is back on prod again. 

Let's reclose this once we get bug 767108 sorted out.

Comment 16

[Matt Brandt [:mbrandt]]

Bug 767108 has been remedied - once again thanks everyone for rocking out this bug.

Comment 17

[Matt Brandt [:mbrandt]]

Bumping back to QA verified

Comment 18

[James Socol [:jsocol, :james]]

Since the fix is in production I'd like to remove this from the websites security group and make it public. Since it's sec-high, I want to check first, though. Matt F--are we OK to open this up now that it's VERIFIED FIXED in prod?

Comment 19

[Matt Fuller :mfuller]

I'm going to defer that to :curtisk, I'll ping him tomorrow to make sure. Thanks again for working on it

Comment 20

[Matt Fuller :mfuller]

I've confirmed that if this has been fixed (which it has) we can go ahead and remove it from the group.