Skip to content
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

Forbid srcdoc frames with inner CSP meta tag #104

Merged
merged 4 commits into from
Jun 20, 2023
Merged

Conversation

weizman
Copy link
Member

@weizman weizman commented Jun 19, 2023

should address #94, #90 and probably some other future crap too.

CSP can prevent Snow from running in new documents, which specifically srcdoc iframes can leverage.

This PR removes the ability to create srcdoc frames with meta CSP tags by assuming that this technique has no real world usage other than malicious.

This was referenced Jun 19, 2023
@weizman weizman changed the title attempt to remove csp meta tag from srcdoc frames Forbid srcdoc frames with inner CSP meta tag Jun 19, 2023
@mmndaniel
Copy link
Contributor

Ah, I hope it's not too dangerous (i.e. significantly increases protected app breakage risk). Anyways, it needs to more work to be secure, because findMetaCSP relays on prototype chain, so:
Object.defineProperty(Element.prototype, 'attributes', { value: [] });
or
Object.defineProperty(Attr.prototype, 'value', { value: '' });
Makes the bypass work again.

@weizman
Copy link
Member Author

weizman commented Jun 19, 2023

Not really because the prototype chain is coming from the realm i used for creating natives in the beginning of execution which is the realm of an iframe that was immediately removed from dom forever, so polluting it is between extremely difficult and probably impossible.
You may try to produce a poc if you believe I'm wrong.

@weizman
Copy link
Member Author

weizman commented Jun 19, 2023

And yes, before merging i will make sure major websites don't suffer from this, but I highly doubt it.
Do you see any other issues with this approach?

@mmndaniel
Copy link
Contributor

Check these out:

Object.defineProperty(Element.prototype, 'attributes', { value: [] });
var d = document.createElement('div');
                testdiv.appendChild(d);
                d.innerHTML = `
    <iframe
        srcdoc="
        <meta http-equiv='Content-SecuriTy-Policy' content=&quot;script-src 'nonce-pwnd' ;&quot;>
            <iframe src=&quot;javascript:haha&quot;>
            </iframe>
        <script nonce=&quot;pwnd&quot;>frames[0].alert(1);</script>">
    </iframe>`
Object.defineProperty(Attr.prototype, 'value', { value: '' });
var d = document.createElement('div');
                testdiv.appendChild(d);
                d.innerHTML = `
    <iframe
        srcdoc="
        <meta http-equiv='Content-SecuriTy-Policy' content=&quot;script-src 'nonce-pwnd' ;&quot;>
            <iframe src=&quot;javascript:haha&quot;>
            </iframe>
        <script nonce=&quot;pwnd&quot;>frames[0].alert(1);</script>">
    </iframe>`

@weizman
Copy link
Member Author

weizman commented Jun 20, 2023

You're so right, this is one detail I missed with safe natives handling, I used a native createElement function but called it on the main vulnerable document, meaning nodes created that way were in fact vulnerable to prototype pollution.
fixed f0aed38

@weizman weizman marked this pull request as ready for review June 20, 2023 13:34
@weizman
Copy link
Member Author

weizman commented Jun 20, 2023

Checked against major websites, seems to work perfectly fine, merging

@weizman weizman merged commit 28049dc into main Jun 20, 2023
2 checks passed
@weizman weizman deleted the remove-srcdoc-csp branch June 20, 2023 13:38
@weizman weizman mentioned this pull request Jul 17, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants