1250114 - XSS possible in extensions calling global/tabs.html.tmpl if tab.link is user-controlled

Status: RESOLVED FIXED

(Bugzilla :: User Interface, defect)

Description

[Andrew]

Attachment: 2016-02-22-140649.png

User Agent: Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/48.0.2564.116 Safari/537.36

Steps to reproduce:

Hi,
I would like to report Cross site scripting vulnerability.

Steps to reproduce : 
1. Navigate to: https://bugzilla.mozilla.org/page.cgi?id=productdashboard.html&product=Participation%20Infrastructure&tab=components&bug_status=%27-prompt(%27xss%27)-%27
2. Switch to any tab ("Recents", "Components/Versions", "Duplicates", etc)


Actual results:

Javascript get executed.


Expected results:

Javascript must not been executed :)

Comment 1

[Frédéric Buclin]

Confirmed. BMO-specific.

Comment 2

[Dylan Hardison [:dylan] (he/him)]

Very unfortunate. :(

I think this is the fault of global/tabs.html.tmpl. 
I think that is only used by productdashboard and the search tabs. Search tabs do not allow user-generated content
into the tabs array.

I note that global/tabs.html.tmpl exists in the filterexceptions.pl -- which I think adds insult to injury.

Working on a fix next.

Comment 3

[Dylan Hardison [:dylan] (he/him)]

Attachment: 1250114_1.patch

filterexceptions isn't the cause of this error. The value in question was already filtered -- for HTML content.
We need to filter tabs.link for JS and then for HTML.

Alternatively, we could remove the onclick handler, as it seems to be pretty useless.

Comment 4

[:glob ✱]

Comment on attachment 8721968 [details] [diff] [review]
1250114_1.patch

Review of attachment 8721968 [details] [diff] [review]:
-----------------------------------------------------------------

r=glob

Comment 5

[Frédéric Buclin]

This bug has been introduced by bug 321556 in 2.23.4.

Comment 6

[Gervase Markham [:gerv]]

Yeah, filterexceptions can tell if values are filtered, but it can't tell if they are filtered the right way :-| Not sure if we'd be able to make it that smart.

Gerv

Comment 7

[Dylan Hardison [:dylan] (he/him)]

We definitely can not -- but we can prevent this and future bugs by a having a good CSP policy.

Comment 8

[Frédéric Buclin]

Actually, this is not a security bug as in a vanilla Bugzilla installation we only have two places where we call global/tabs.html.tmpl, and none of them passes user-controlled data to tabs. So this is rather a security improvement. No need for a secadv, meaning that it can be checked in immediately (after appropriate a+, of course).

Comment 9

[Dylan Hardison [:dylan] (he/him)]

To ssh://[e-mail]/bugzilla/bugzilla.git
   ee24e1e..01ad7ac  4.4 -> 4.4
To ssh://[e-mail]/bugzilla/bugzilla.git
   6c705e8..a59f1e9  5.0 -> 5.0
To ssh://[e-mail]/bugzilla/bugzilla.git
   a35c986..54f8e93  master -> master