Closed
Bug 1305713
Opened 8 years ago
Closed 8 years ago
BMO: Persistent XSS via Git commit messages in comments
Categories
(bugzilla.mozilla.org :: Extensions, defect)
Tracking
()
RESOLVED
FIXED
People
(Reporter: jwkbugzilla, Assigned: dkl)
References
()
Details
(Keywords: sec-critical, wsec-xss, Whiteboard: [reporter-external] [web-bounty-form] [verif?])
Attachments
(1 file, 1 obsolete file)
3.20 KB,
patch
|
dylan
:
review+
|
Details | Diff | Splinter Review |
BMO comments linkify Git commit messages like the following: To https://github.com/foobar.git 1234..4321 master -> master This is implemented as a format hook in extensions/BMO/Extension.pm. Format hooks run before HTML entities are escaped in the text and are responsible for escaping their results themselves - this particular hook fails to do it. So the following text in a comment will run JavaScript code: > To https://github.com/foobar.git > 1234..4321 master<iframe/onload=alert(document.domain)> -> master Note that I quoted the text here to make sure the hook isn't triggered. Without quoting it would display an alert saying "bugzilla.mozilla.org". You don't need to create a new bug in order to test this, entering the text into a comment and switching to the preview tab will already display the alert. Note that there are two branches in the code processing Git commit messages (github.com and git.mozilla.org). Both of them are vulnerable and the following text will trigger the bug in the other branch: > To git@git.mozilla.org/foobar.git > 1234..4321 master<iframe/onload=alert(document.domain)> -> master
Flags: sec-bounty?
Comment 1•8 years ago
|
||
Wladimir -- I'm going to post the text of bug 1305717 into here. Although they are distinct bugs, they are close enough that they can probably be considered all part of the same problem.
Comment 2•8 years ago
|
||
I'm also going to use bug 1305717 as the PoC bug, since I can't seem to get into our test site at the moment.
Comment 3•8 years ago
|
||
From bug 1305717: -- BMO comments rewrite links to BMO repositories on git.mozilla.org to point to GitHub instead: https://git.mozilla.org/?p=webtools/bmo/bugzilla.git;a=tree [github] [github] While the text stays unchanged here, target of the link has been modified. This is implemented as a format hook in extensions/BMO/Extension.pm. Format hooks run before HTML entities are escaped in the text and are responsible for escaping their results themselves - this particular hook fails to do it. So the following link will run JavaScript code if you remove the space from it: https://git.mozilla.org/?p=webtools/bmo/bugzilla "><iframe/onload=alert(document.domain)>.git;a=tree Without the space this link would display an alert saying "bugzilla.mozilla.org". You don't need to create a new bug in order to test this, entering the text into a comment and switching to the preview tab will already display the alert.
Updated•8 years ago
|
Group: websites-security → bugzilla-security
Component: Other → General
Keywords: sec-critical,
wsec-xss
Product: Websites → bugzilla.mozilla.org
Version: unspecified → Production
Updated•8 years ago
|
Component: General → Extensions: Other
Assignee | ||
Updated•8 years ago
|
Assignee: nobody → dkl
Status: NEW → ASSIGNED
Component: Extensions: Other → General
Updated•8 years ago
|
Component: General → Extensions: BMO
Comment 5•8 years ago
|
||
I note this is not exploitable on bugzilla-dev under the modal UI. If only I had gotten that the CSP changes out a little faster.
Comment 6•8 years ago
|
||
I believe this bug has existed since bug 982788
Assignee | ||
Comment 7•8 years ago
|
||
Found some other places that need to be escaped at well :( dkl
Attachment #8795356 -
Flags: review?(dylan)
Comment 8•8 years ago
|
||
I can confirm this was possible as of bug 982788 which was committed without review. It should have been caught in by me in either the review for bug 1281479 or bug 1286960.
Comment 9•8 years ago
|
||
Comment on attachment 8795356 [details] [diff] [review] 1305713_1.patch Review of attachment 8795356 [details] [diff] [review]: ----------------------------------------------------------------- still vulnerable to "https://git.mozilla.org/?p=webtools/bmo/bugzilla" ><iframe/onload=alert(document.domain)>.git;a=tree
Attachment #8795356 -
Flags: review?(dylan) → review-
Assignee | ||
Comment 10•8 years ago
|
||
Attachment #8795356 -
Attachment is obsolete: true
Attachment #8795378 -
Flags: review?(dylan)
Comment 11•8 years ago
|
||
Comment on attachment 8795378 [details] [diff] [review] 1305713_2.patch Review of attachment 8795378 [details] [diff] [review]: ----------------------------------------------------------------- r=dylan
Attachment #8795378 -
Flags: review?(dylan) → review+
Reporter | ||
Comment 12•8 years ago
|
||
(In reply to Dylan Hardison [:dylan] from comment #8) > I can confirm this was possible as of bug 982788 which was committed without > review. The other issue which was marked as a duplicate landed in bug 1282931 - proper review and everything. (In reply to David Lawrence [:dkl] from comment #7) > Found some other places that need to be escaped at well :( From what I can tell these aren't exploitable. Not that I'm against proper escaping everywhere.
Assignee | ||
Comment 13•8 years ago
|
||
To https://github.com/mozilla-bteam/bmo.git 8affa3e..4ac7c1e master -> master
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Comment 15•8 years ago
|
||
(In reply to Wladimir Palant from comment #12) > (In reply to Dylan Hardison [:dylan] from comment #8) > > I can confirm this was possible as of bug 982788 which was committed without > > review. > > The other issue which was marked as a duplicate landed in bug 1282931 - > proper review and everything. > Both bug 1281479 or bug 1286960 were proper reviews, but I was blind to the lack of html_quote(). I think dkl's patch on this of adding that everywhere is good defensive programming. An even better defense is https://bugzilla.mozilla.org/show_bug.cgi?id=1286290, which I will be expanding to cover both versions of show_bug.cgi.
Updated•8 years ago
|
Flags: sec-bounty? → sec-bounty+
Updated•5 years ago
|
Component: Extensions: BMO → Extensions
You need to log in
before you can comment on or make changes to this bug.
Description
•