XSS in data-target attribute
lpilorz opened this issue · 44 comments
The data-target attribute is vulnerable to Cross-Site Scripting attacks:
<script src="https://ajax.googleapis.com/ajax/libs/jquery/2.2.4/jquery.min.js"></script>
<script src="http://maxcdn.bootstrapcdn.com/bootstrap/3.3.6/js/bootstrap.min.js"></script>
<button data-toggle="collapse" data-target="<img src=x onerror=alert(0)>">Test</button>I guess the fix for this specific issue could be changing getTargetFromTrigger function:
return $(target)to:
return $(document.querySelector(target))but it seems the same problem is present in other places too.
Here is another example:
<a href="<img src=x onerror=alert(0)>" data-dismiss="alert">Test</a>
return $(document.querySelector(target))
I don't think that's a viable option compatibility-wise. Bootstrap 3 supports IE8, and its CSS selector support differs from jQuery's: http://caniuse.com/#feat=queryselector
Based on https://bugs.jquery.com/ticket/11290, I guess we could try something like:
if (/\s*</.test(target)) {
return $()
}@dmethvin Sorry to trouble you, but does this sound like a good workaround?
(It's a shame jQuery doesn't have an "only interpret this as a selector, never as HTML" API.)
You could use $(document).find(target) instead, that won't be interpreted as HTML.
How is this cross-site? Where does the HTML come from and how does the attacker control it?
This was found in an application where data-target was based on user input and only passed through standard HTML entities encoding. There is no reason why data-target should interpret HTML so while not impacting many applications it should be fixed in my opinion.
Bootstrap 3 is no longer being officially developed or supported.
All work has moved onto our next major release, v4. As such, this issue or pull request is being closed as a "won't fix." For additional help and support, we recommend utilizing our community resources. Thanks for your understanding, and see you on the other side of v4!
<3,
@mdo and team
I know this one's closed, but what's the status for v4? I can see that it is still affected by this vulnerability:
https://github.com/twbs/bootstrap/blob/v4-dev/js/src/util.js#L120
I recommend to mitigate it by applying @dmethvin's proposal. (Also see comits on @vanduynslagerp's fork.)
Should I prepare a pull request? Or should I open a new issue for v4?
You could make a PR against v3-dev branch too if you want.
Let's start off with a test case: http://jsbin.com/qalekeroke/edit?html,output
Thank you for getting the fix into v4-dev so quickly. And for initiating back-port to v3-dev.
Just some remarks regarding the fix in Util.getSelectorFromElement() on v4-dev:
The code in there is not XSS-prone anymore:) However, it returns a plain selector string. All callers of Util.getSelectorFromElement() (well, the 10 code locations that I could find) then pass the selector to the jQuery function. That is (conditional execution aside) they do something like:
$(Util.getSelectorFromElement(e))
This is secure, only if both of these guarantees hold:
Util.getSelectorFromElement()only ever returns strings that are valid CSS selectors. (Which is true now.)- The
jQueryfunction, when invoked with any string argument that is a valid CSS selector, will never interpret that string as an HTML fragment.
If guarantee 2 is broken, then guarantee 1 becomes pointless. I have tried to craft a respective CSS selector today, luckily without success so far. I've only tested against 2 recent jQuery versions though. I believe that past jQuery versions applied more dubious magic when it came to distinguishing CSS selectors from HTML fragments.
Long story short:
Just wondering if Util.getSelectorFromElement() might be safer, if it returned a jQuery object rather than a plain string. I could not find any usage of Util.getSelectorFromElement() that would prevent that. It might even be slightly more efficient for complex CSS selectors (which are currently evaluated twice, wherever that utility function is used).
If you think that makes sense, I can try to refactor it, and prepare a pull-request...
@meeque As of version 1.9 the jQuery method never interprets a string as HTML unless it starts with a less-than sign, potentially preceded by spaces. It looks like the latest v3 doesn't support anything older than jQuery 1.9.1.
@dmethvin Yupp, that's the current situation. The question remains:
Can anyone craft a valid CSS selector that starts with a less-then sign? I failed at it :) -- doesn't mean it cannot be done.
I don't think it would be possible. A literal less-than < character can't start a selector. It would have to be escaped which means it wouldn't be interpreted as HTML.
@dmethvin OK, let's leave it at that :-)
We're good until CSS selectors spec introduces a less-then operator that may be used at the beginning of the selector. This may or may not happen in the future, but worrying about it now is probably paranoid.
Are these fixes available in any v3 stable release?
not currently but it seems we'll release a new version of v3, but when that's an another question 😄
Excuse me, but could anyone explain why this is considered XSS? If an attacker can control the value of a data-target attribute, inject a new data-target attribute into an existing element, or even worse, if they can inject a new element with a data-target attribute, isn’t that by itself the XSS vulnerability, rather than the fact that Bootstrap will then interpret the injected value as HTML? Of course, it never hurts to have an extra layer of protection, but it seems to me that the XSS label is misplaced here.
@astiob This would be a naive example of code vulnerable to XSS with Bootstrap:
<button data-toggle="collapse" data-target="<?=htmlspecialchars($_GET['x']);?>">Test</button>
"An attacker can control the value of a data-target attribute" - this does not cause XSS usually, but becomes vulnerable with Bootstrap. In my opinion this should be either fixed or documented.
If the attacker can insert a <button> element into the document with a specially crafted data-target attribute, why wouldn't they just insert a <script> tag or a <img onerror=> instead?
The attacker does not inject the button element, in the above example it's a part of the vulnerable page. I've setup up an example vulnerable page to explain:
http://team.runic.pl/bootstrap.php?x="><script>alert(document.domain)</script>
escaped to HTML entities so does not execute the script
http://team.runic.pl/bootstrap.php?x=<img src=x onerror=alert(document.domain)>
due to Bootstrap behaviour executes injected script on button click
A similar situation (control over data-target attribute alone) is very rare, but it happened in at least one real-life banking app.
As this is an XSS vulnerability, is there any chance with the commit made on the pull request below actually going into a patched 3.3.7 or 3.3.8?
#23687
This has been found as a 'Serious' vulnerability within the McAfee SECURE scan our company uses. We have applied the fix above to our use of 3.3.7, but as the scan is looking for versions which have known vulnerabilities, it could cause confusion down the road if we ever have an external security audit beyond this.
We plan to move to 4.0 only when out of beta.
Thanks!
For reference, I was in a similar situation, and what I did to satisfy the security audit tool was import the v3.4.0-dev branch as a Git submodule in my project and use its Less and JavaScript sources instead of those from the v3.3.7 release. It contains the fix, and it reports 3.4.0 as the version number, so the tool is happy. (My application was not vulnerable anyway, but I figured it would be easier and more future-proof to do this than to persuade the auditors to disable this test as a false positive.)
Has anyone started the process of getting a CVE for this issue?
@mdo: ping again. See #20184 (comment)
Is the vulnerability related to XSS fixed in 3.3.7/
No. For v3, it’s currently only fixed in the v3.4.0-dev branch.
Should we download bootstrap v4.0 for XSS vulnerability
Any chance that v3.4.0 with the fix could be released in the near future? (sorry, I don't know bootstrap release process.)
@kipphoward That is unlikely #20631
Discussion of a 3.4 release seems to have moved to #25679 (for anyone else who finds this issue first).
A key update from @mdo yesterday:
The team has spent a lot of time cleaning up that branch and getting it in the right state. Hang tight, should have good news soon.
#25679 (comment)
Hard to follow along as I can't find the v3.4.0-dev branch in GitHub. Maybe it's at v3-dev-browserstack?
Sorry if I am asking a silly question. Is bootstrap v3.3.7 safe and secure to use if "data-target" attribute is unused?
Apart from data-target also href attribute can be vulnerable (see #20184 (comment))
Hi, Can i have this issue fixed in 3.3.7 itself as i cant update it to 3.4 ? Updating to 3.4 is completely distorting our website.
@imran2489 That would have to be a 3.3.8 as per semver. It would require branching from the 3.3.7 tagged commit then applying the fix.
Yes but there is no such version.
There should be no reason why you cannot upgrade from v3.3.7 to v3.4.0. The changes to our compiled CSS between the two releases can be seen at https://github.com/twbs/bootstrap/compare/v3.4.0..v3.3.7#diff-6972cbb37f0ec48771217d4915ea7e6f. Most of the changes seen there are reordering properties—there should be no changes to selectors, components, etc other than those (and the addition of gutterless rows).
is this considered a xss vulnerabilitie due to bootstrap version? https://jsbin.com/vasitugato/edit?html,output
can anyone help me undestand it pleasee?
does it need to be into <button and to work or just paste the xss in random data-target??? i am seeing many websites with this vulnerable library but can't exploit them and report them because i am doing something wrong maybe :_(
The data-target attribute is vulnerable to Cross-Site Scripting attacks:
<script src="https://ajax.googleapis.com/ajax/libs/jquery/2.2.4/jquery.min.js"></script> <script src="http://maxcdn.bootstrapcdn.com/bootstrap/3.3.6/js/bootstrap.min.js"></script> <button data-toggle="collapse" data-target="<img src=x onerror=alert(0)>">Test</button>
how to test for it can you explain I found it