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

Add security headers to nextjs config to prevent XSS attacks and clickjacking #4841

Open
geleeroyale opened this issue Oct 14, 2024 · 11 comments
Assignees

Comments

@geleeroyale
Copy link
Collaborator

We got some reports in the past that it is possible to run a "clickjacking" attack against giveth.io

(essentially people could embed the whole site in an iframe and display it on their site, changing some links in their portal to their own)

Now this is known and has been like that for a long time without being exploited (its also quite hard to do something useful) ... regardless, we should probably fix it.

https://dev.to/theinfosecguy/how-to-protect-your-nextjs-website-from-clickjacking-2jbg
https://nextjs.org/docs/pages/building-your-application/configuring/content-security-policy

@github-project-automation github-project-automation bot moved this to New Issues in All-Devs Oct 14, 2024
@kkatusic kkatusic self-assigned this Oct 15, 2024
@kkatusic kkatusic moved this from New Issues to In Progress in All-Devs Oct 15, 2024
@kkatusic
Copy link
Collaborator

@geleeroyale can you check do we have some default vercel headers serving Giveth app? thx

@geleeroyale
Copy link
Collaborator Author

geleeroyale commented Oct 16, 2024

@kkatusic the frontend config should be located in next-config.ts and we have CORS configured on the reverse proxy to access impact-graph.

Apart from that I think we did not set any headers.

Here is the CORS config we are using:

# CORS Config Block Directive
(cors) {
    @cors_preflight {
        method OPTIONS
    }
    @corsOrigin {
        header_regexp Origin ^https?://([a-zA-Z0-9-]+\.)*vercel\.app$|^https?://localhost(:[0-9]+)?$|^https?://({$DOMAIN_WHITELIST})$
    }

    handle @cors_preflight {
        header {
            Access-Control-Allow-Origin "{http.request.header.Origin}"
            Access-Control-Allow-Credentials true
            Access-Control-Allow-Headers "*"
            Access-Control-Allow-Methods "GET, POST, PUT, PATCH, DELETE"
            Access-Control-Max-Age "3600"
            Vary Origin
            defer
        }
        respond "" 204
    }

    handle @corsOrigin {
        header {
            Access-Control-Allow-Origin "{http.request.header.Origin}"
            Access-Control-Allow-Credentials true
            Access-Control-Expose-Headers "*"
            Vary Origin
            defer
        }
    }
}

(cors-sse) {
    header {
        Access-Control-Allow-Origin "*"
        Access-Control-Allow-Credentials true
        Access-Control-Expose-Headers "*"
        Vary Origin
        defer
    }
}

@kkatusic
Copy link
Collaborator

@geleeroyale ok and that's ok, but look at header that you get from giveth.io and look, what extra we defined inside next.config.js file:

headers: [
but same never loads to browser

@geleeroyale
Copy link
Collaborator Author

Yes - It looks like its not properly configured. Please take a deeper look into this.

@kkatusic
Copy link
Collaborator

@geleeroyale can you test inside preview link these security headers:

https://giveth-dapps-v2-git-feat-securityheaders-givethio.vercel.app/

@geleeroyale
Copy link
Collaborator Author

Sorry - missed this. Will take a look.

@geleeroyale
Copy link
Collaborator Author

geleeroyale commented Nov 30, 2024

@kkatusic Sorry for taking so long - I reviewed your PR and it is a great improvement - specifically as requested X-Frame Options (disallows using giveth in iframes) and content security policy (prevents against cross site scripting attacks) have now been implemented.

Before:
image

After:
image

1 - I did not use these testing sites before and we might want to try to reach A grade by implementing the rest of recommended security headers
2 - I strongly suggest we take your experience and knowledge to all our other products and make this a standard in all our applications across the Giveth Galaxy and GM

Edit: Results are taken from https://securityheaders.com - also a good resource is Mozilla Observatory for quick checks (https://developer.mozilla.org/en-US/observatory)

@kkatusic
Copy link
Collaborator

kkatusic commented Dec 2, 2024

Thx @geleeroyale for reply, can you check now with new ones added headers:

https://giveth-dapps-v2-git-feat-securityheaders-givethio.vercel.app/

thx

@geleeroyale
Copy link
Collaborator Author

You are a goddamn hero!! A+ score

Image

Please consider doing this for our other products as well 💜

@kkatusic
Copy link
Collaborator

kkatusic commented Dec 3, 2024

You are a goddamn hero!! A+ score

Image

Please consider doing this for our other products as well 💜

thx @geleeroyale.

@kkatusic
Copy link
Collaborator

kkatusic commented Dec 3, 2024

For QA people, I tested donation, checked values and seems that everything is working fine, maybe some external api url request can fail, just to notice here if it is blocked by newly added headers.

@kkatusic kkatusic moved this from In Progress to QA in All-Devs Dec 3, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: QA
Development

No branches or pull requests

2 participants