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.

Sign up for GitHub

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

Jump to bottom

Stop Bypass X-Frame-Options #8324

Closed
frankchen07 opened this issue Jan 28, 2021 · 1 comment · Fixed by #8335
Closed

Stop Bypass X-Frame-Options #8324

frankchen07 opened this issue Jan 28, 2021 · 1 comment · Fixed by #8335
Assignees
@gdixon
Labels
gpg - infrastructure
Milestone
GPG [2021.01.19 -...

Comments

@frankchen07
Copy link
Contributor

Who

When a malicious user can

What

bypass the X-Frame-Options header and create clickjacking on the whole domain

Why

we should stop this in its tracks

Details

POC :

<!DOCTYPE html>
<html>
<head>
        <meta charset="UTF-8">
        <meta name="viewport" content="width=device-width, initial-scale=1.0">
        <meta name="description" content="X-Frame-Bypass: Web Component extending IFrame to bypass X-Frame-Options: deny/sameorigin">
        <title>X-Frame-Bypass Web Component Demo</title>
        <style>
                html, body {
                margin: 0;
                padding: 0;
                height: 100%;
                        overflow: hidden;
                }
                iframe {
                        display: block;
                        width: calc(100% - 40px);
                        height: calc(100% - 40px);
                        margin: 20px;
                }
                img {
                        position: absolute;
                        top: 0;
                        right: 0;
                }
        </style>

        <script src="https://unpkg.com/@ungap/custom-elements-builtin"></script>
        <script src="x-frame-bypass.js" type="module"></script>
</head>
<body>
        <h1> X-FRAME PROTECTION BYPASSED </h1>
        <iframe is="x-frame-bypass" src="https://gitcoin.co/"></iframe>

</body>
</html>

Implement Content-Security-Policy: frame-ancestors 'self' is better, because it checks all frame ancestors. We should implement a CSP header to avoid these sort of attacks.

We will also have to add https://content-security-policy.com to the nginx config file.
@gdixon
Copy link
Contributor

gdixon commented Jan 29, 2021 •
edited
Loading

@frankchen07 @thelostone-mc @octavioamu Is this something that needs to be handled by nginx?

Currently we set X-Frame-Options inside the Django settings: I think we should try to keep as much of our header configuration there as possible, so that it's all together and part of the deploy. If we set the CSP in nginx it would be sent with every request but we would have to manage it outside of this repo. We only need to protect the html content, which is all being served by Django? Is there currently any way for a user to upload files anywhere in the system (that might be rendered as html & accessed directly)? Do we have anything that accepts SVG's (if there is, is there a sanitise method in place)? If we're okay on those fronts, then we shouldn't need to be sending the CSP with content that we know is safe (but it wouldn't hurt to do so)!

This looks like a solid solution: https://github.com/mozilla/django-csp/

If we're all okay with the caveats above, then I have this ready to push, but I'm happy to delete it if it does need to go in the conf, we're better to be safe than sorry when it comes to security configuration.

--

I've also tested a more complete (but basic) policy (frame_ancestors & script_src) and a lot of the content is blocked unless I include unsafe-inline and unsafe-eval (because of the way vuejs carries out rendering from a html template). I'll open another issue regarding this because I think we will need to have a discussion around how we go about implementing a more thorough CSP safely.

gdixon added a commit to gdixon/web that referenced this issue Feb 1, 2021
@gdixon
Adds CSP header (frame-ancestors 'self') to django settings (gitcoinc…
ef9a1c7 
…o#8324)
gdixon added a commit to gdixon/web that referenced this issue Feb 1, 2021
@gdixon
Adds CSP header (frame-ancestors 'self') to django settings (gitcoinc…
7827047 
…o#8324)
gdixon added a commit to gdixon/web that referenced this issue Feb 1, 2021
@gdixon
Adds CSP header (frame-ancestors 'self') to django settings (gitcoinc…
8b9faef 
…o#8324)
@gdixon gdixon mentioned this issue Feb 1, 2021
Merged
thelostone-mc pushed a commit that referenced this issue Feb 2, 2021
@gdixon
Adds CSP header (frame-ancestors 'self') to django settings (#8324) (#…
224a0b5 
…8335)
@gitcoinco gitcoinco deleted a comment from gitcoinbot Jun 18, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@gdixon gdixon

Labels
gpg - infrastructure
Projects
None yet
Milestone
GPG [2021.01.19 - 2021.02.02]
Development

Successfully merging a pull request may close this issue.

Adds CSP header (frame-ancestors 'self') to django settings (#8324) gdixon/web
3 participants
@gdixon @frankchen07 and others