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

CSRF #477

Open
moshetanzer opened this issue Jun 15, 2024 · 16 comments
Open

CSRF #477

moshetanzer opened this issue Jun 15, 2024 · 16 comments
Labels
awaiting details Waiting for feedback from the issue author, i.e. reproduction bug Something isn't working

Comments

@moshetanzer
Copy link
Contributor

moshetanzer commented Jun 15, 2024

Hi,

Thanks again for your great library.

Having this issue wonder if you could point me in the right direction.

I have CSRF protection enabled for my login page. My app is hosted on Vercel.

Seems to be that the first time out customer comes back to app, the CSRF always fails and requires a page reload for it to work.

It is an internal app only used I would say weekly. Could it be something with vercel's Serverless architecture. Seems strange.

Any suggestions?

Latest version
nuxt-security:
nuxt:

@moshetanzer moshetanzer added the bug Something isn't working label Jun 15, 2024
@Baroshem
Copy link
Owner

That is an interesting question @Morgbn would you have some ideas about it?

@moshetanzer
Copy link
Contributor Author

Have disabled for now... would love a solution

@moshetanzer
Copy link
Contributor Author

Also been a pain since when you disable csurf in nuxt config. Whole app breaks since car is not found...

@christie-hill-za
Copy link

I also just noticed this issue with a form. I am hosting our Nuxt site on Azure Static Web Apps.

Not sure what the cause could be, but if I have more info, I will share here.

@Baroshem
Copy link
Owner

Don't have answer now. I would love to get feedback from @Morgbn about it as he knows the stuff :)

@moshetanzer
Copy link
Contributor Author

Hi,

I think that maybe a warning should be put on the website (even thought it is by default disabled) that CSRF doesnt work properly with serverless functions.

@Morgbn
Copy link
Contributor

Morgbn commented Jun 19, 2024

Hello, I recently noticed the same thing on a project hosted on Cloudfare
I need to investigate,
I'll get back to you as soon as I know more!

@moshetanzer
Copy link
Contributor Author

moshetanzer commented Jun 19, 2024

Issue is simple. Functions should be considered starless due to their cold state nature. Which means any solution using the same method cannot work. We have to change this CSRF to the Double submit cookie pattern.

UPDATE: Think it will be better and easier to just go with the Origin method. @Baroshem should we get the host from a ENV? Or Host or X-forwards host. Issue is that I think x forwarded is disabled by default?

If I have a bit of time 😇 will open a PR.

@Baroshem
Copy link
Owner

Baroshem commented Jul 4, 2024

Hey @Morgbn any findings in your testing? :)

@moshetanzer
Copy link
Contributor Author

Hey @Baroshem,

Sorry i havent managed to do a PR, might be quicker if you want just to add this as csrfOrigin so that the token can still be used for those not hosting on serverless.

Found this the other day it is from the oslo/request package. It use a safe header. Only issue with it is one of the version of internet explorer which i am not convinced is such a big issue.

This method is suggested also in the owasp in defence in deep" - https://cheatsheetseries.owasp.org/cheatsheets/Cross-Site_Request_Forgery_Prevention_Cheat_Sheet.html#using-standard-headers-to-verify-origin

export function verifyRequestOrigin(origin: string, allowedDomains: string[]): boolean {
	if (!origin || allowedDomains.length === 0) return false;
	const originHost = safeURL(origin)?.host ?? null;
	if (!originHost) return false;
	for (const domain of allowedDomains) {
		let host: string | null;
		if (domain.startsWith("http://") || domain.startsWith("https://")) {
			host = safeURL(domain)?.host ?? null;
		} else {
			host = safeURL("https://" + domain)?.host ?? null;
		}
		if (originHost === host) return true;
	}
	return false;
}

function safeURL(url: URL | string): URL | null {
	try {
		return new URL(url);
	} catch {
		return null;
	}
}

@moshetanzer
Copy link
Contributor Author

Any updates on this?

@Baroshem
Copy link
Owner

Still waiting for the feedback from @Morgbn :)

@moshetanzer
Copy link
Contributor Author

Wasn't suggesting to remove his package. Rather just add a this as 'corsOrigin'

@Baroshem Baroshem added the awaiting details Waiting for feedback from the issue author, i.e. reproduction label Jul 16, 2024
@Baroshem
Copy link
Owner

Baroshem commented Jul 16, 2024

@moshetanzer yeah, I understand. I am just asking for the opinion and guidance from the author of the nuxt-csurf package as this technically could be implemented there :)

@Morgbn let me know if you found something

@moshetanzer
Copy link
Contributor Author

moshetanzer commented Sep 19, 2024

Congrats on major!

Hey @Baroshem seems like a pity to ship major security package without a working CSRF protection. Why don’t we just add in the meantime my above suggestion as csrf-origin or as a type on csrf.

@Baroshem
Copy link
Owner

Tagging @Morgbn for opinion :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
awaiting details Waiting for feedback from the issue author, i.e. reproduction bug Something isn't working
Projects
None yet
Development

No branches or pull requests

4 participants