Skip to content

Forbid srcdoc frames with inner CSP meta tag#104

Merged
weizman merged 4 commits intomainfrom
remove-srcdoc-csp
Jun 20, 2023
Merged

Forbid srcdoc frames with inner CSP meta tag#104
weizman merged 4 commits intomainfrom
remove-srcdoc-csp

Conversation

@weizman
Copy link
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
@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.

2 participants