Closed Bug 1250114 Opened 8 years ago Closed 8 years ago

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

Categories

(Bugzilla :: User Interface, defect)

Product:
Bugzilla
Component:
User Interface
Version:
2.23.4
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

()

Status:
RESOLVED FIXED
Milestone:
Bugzilla 4.4

People

(Reporter: buglloc, Assigned: dylan)

References

Details

(Keywords: sec-moderate, wsec-xss)

Attachments

(2 files)

2016-02-22-140649.png
707.87 KB, image/png
Details
1250114_1.patch
723 bytes, patch
glob
: review+
Details | Diff | Splinter Review
Attached image 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 :)
Confirmed. BMO-specific.
Assignee: general → nobody
Status: UNCONFIRMED → NEW
Component: Bugzilla-General → Extensions: ProductDashboard
Ever confirmed: true
Product: Bugzilla → bugzilla.mozilla.org
QA Contact: default-qa
Version: unspecified → Production
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.
Assignee: nobody → dylan
Summary: XSS in page.cgi (bug_status parameter) → XSS in Product Dashboard (bug_status parameter)
Attached patch 1250114_1.patch — — Splinter Review 
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.
Attachment #8721968 - Flags: review?(dkl)
Comment on attachment 8721968 [details] [diff] [review]
1250114_1.patch

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

r=glob
Attachment #8721968 - Flags: review?(dkl) → review+
Component: Extensions: ProductDashboard → User Interface
Product: bugzilla.mozilla.org → Bugzilla
QA Contact: default-qa
Summary: XSS in Product Dashboard (bug_status parameter) → XSS in global/tabs.html.tmpl (tab.link not filtered correctly)
Target Milestone: --- → Bugzilla 5.0
Version: Production → 4.2
Flags: sec-bounty?
This bug has been introduced by bug 321556 in 2.23.4.
Status: NEW → ASSIGNED
Depends on: 321556
Flags: blocking5.0.3?
Flags: blocking4.4.12?
Flags: approval?
Flags: approval5.0?
Flags: approval4.4?
Summary: XSS in global/tabs.html.tmpl (tab.link not filtered correctly) → [SECURITY] XSS in global/tabs.html.tmpl (tab.link not filtered correctly)
Target Milestone: Bugzilla 5.0 → Bugzilla 4.4
Version: 4.2 → 2.23.4
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
We definitely can not -- but we can prevent this and future bugs by a having a good CSP policy.
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).
Flags: blocking5.0.3?
Flags: blocking4.4.12?
Summary: [SECURITY] XSS in global/tabs.html.tmpl (tab.link not filtered correctly) → XSS possible in extensions calling global/tabs.html.tmpl if tab.link is user-controlled
Flags: approval?
Flags: approval5.0?
Flags: approval5.0+
Flags: approval4.4?
Flags: approval4.4+
Flags: approval+
Blocks: 1258124
To ssh://gitolite3@git.mozilla.org/bugzilla/bugzilla.git
   ee24e1e..01ad7ac  4.4 -> 4.4
To ssh://gitolite3@git.mozilla.org/bugzilla/bugzilla.git
   6c705e8..a59f1e9  5.0 -> 5.0
To ssh://gitolite3@git.mozilla.org/bugzilla/bugzilla.git
   a35c986..54f8e93  master -> master
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Flags: sec-bounty? → sec-bounty+
Keywords: sec-moderate, wsec-xss
Group: bugzilla-security
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: