Skip to content

This issue was moved to a discussion.

You can continue the conversation there. Go to discussion →

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 protection #82

Closed
mwcampbell opened this issue Jan 18, 2021 · 7 comments
Closed

CSRF protection #82

mwcampbell opened this issue Jan 18, 2021 · 7 comments
Labels

Comments

@mwcampbell
Copy link

Since Remix encourages the use of classic HTML form posts, I wonder if it has any sort of protection against cross-site request forgery. If so, I couldn't find anything about that in the docs.

@mwcampbell
Copy link
Author

I'd still like an answer to this question. I won't be comfortable using Remix for a serious project until I know I can protect users from CSRF. Thanks.

@benwis
Copy link
Contributor

benwis commented Apr 13, 2021

I'll take a stab at answering this. The answer is that, even with classic forms, protection from CSRF falls on you to implement. Remix could do it for you, but since we have a number of implementation choices your mitigation might be different. Still, updating the docs on release would be helpful.

Essentially for CSRF attacks you need three things:
Form Actions. There is an action within the application that the attacker has a reason to induce. This might be a privileged action (such as modifying permissions for other users) or any action on user-specific data (such as changing the user's own password).
Cookie-based session handling. Performing the action involves issuing one or more HTTP requests, and the application relies solely on session cookies to identify the user who has made the requests. There is no other mechanism in place for tracking sessions or validating user requests.
No unpredictable request parameters. The requests that perform the action do not contain any parameters whose values the attacker cannot determine or guess. For example, when causing a user to change their password, the function is not vulnerable if an attacker needs to know the value of the existing password.
Copied from https://portswigger.net/web-security/csrf

So let's talk about Remix here. We definitely have #1 with form actions. Not much to do there.

#2 Cookie based session handling: Remix provides multiple ways to do session handling. If you use cookies, you can protect against this by setting SameSite=Strict on your session cookies. This prevents all requests coming from third party sites from attaching your session cookie, and you're safe. If you're using file based sessions, the session ID is the only thing stored in the cookie, and essentially acts as a CSRF token, since the session ID is also in the file on the server. You probably still need to check the two match when you call an action. Same thing if you implement database session storage.

#3 No unpredictable request parameters: This one is up to you to implement, but you have options. If you're using cookies you should set sameSite=Strict in production, or have your Remix server generate a unique per user session CSRF token, and include that in your form as a hidden field. Then check that its valid in your form action. With file based or db based, you can implement CSRF token checks in your actions and store the token server side.

Either way, assuming you behave responsibly, there is no reason Remix is more vulnerable to CSRF because of regular form actions. As I mentioned earlier though, the docs should be updated to discuss this, and I'm sure they will be once we've hit 1.0

@davidbarratt-drizly
Copy link

davidbarratt-drizly commented Jan 9, 2022

I'm shocked that this web framework uses HTML Forms, but does not protect against CSRF attacks by default. To make matters worse, the documentation does not even mention this. Where it does mention it, it is completely wrong

First, the new logout route is just there to make it easy for us to logout. The reason that we're using an action (rather than a loader) is because we want to avoid CSRF problems by using a POST request rather than a GET request. This is why the logout button is a form and not a link. Additionally, Remix will only re-call our loaders when we perform an action, so if we used a loader then the cache would not get invalidated. The loader is just there in case someone somehow lands on that page, we'll just redirect them back home.

To reiterate for those in the back, using a POST does not in any way prevent a CSRF.

I'm a long time user of frameworks like Symfony who provide CSRF protection out of the box and on by default. You have to explicitly disable the protection on a per-form basis. This is similar to React's built-in XSS protection and their extremely verbose method of bypassing it.

The above comment is also incorrect because it assumes that I'm running a site on the internet rather than perhaps an intranet.

Let's say I build a form for Example, Inc.'s internal intranet and it's hosted at https://deleteemployee.corp.example.com, and it's only accessible if you are on example.com's corporate VPN. Since I know that is the only way to access it, I don't bother to build a login system (why go to that effort?) and Remix is great so I use that for my framework.

Well one day I get an email to my personal email that all of our employees have been deleted from our database! How is that possible?

Well... come to find out, one of our employees went to a competitor's website to do some price comparisons, they had a little snippet on their site that looks like this:

fetch('https://deleteemployee.corp.example.com', {  method: 'POST', body: '...' });

while they weren't able to read the response with JavaScript, the browser happily executed the request since it met the qualifications for a simple request.

There are really only a few ways to protect against this:

  1. You could have the server enforce that incoming POST requests do not meet the criteria for a simple request. For example, by enforcing a Content-Type: application/json header. Of coursse this would force JavaScript to be used for form submission.
  2. Setting a cookie (SameSite=Strict) on the HTML page that has the form, that contains a signed token (a JWT?) that is verified on the POST request.
    a. If you want to support older browsers (SameSite=Lax) you could also include a CSRF token in the form POST (as a hidden field) that is validated against the cookie. This is safe because in my example, even if the browser submitted the cookies, the attacker wouldn't be able to read the HTML page in order to get the hidden form value (though this value would need to be unique per request and tied to the cookie)

If 2 is implemented, it needs to be well documented that the cookie should be excluded from caching decisions. You wouldn't want someone to always bypass the HTTP cache just because they happened to land on a page with a form.

I can't recommend that anyone use this framework until this is fixed. Yes, engineers do (on occasion) need a way to opt-out of these protections, but by default they should be there.

I'm really hoping this is resolved quickly because Remix does seem really great and I'm excited to make something with it!

@mwcampbell
Copy link
Author

Perhaps Remix is relying on the SameSite feature of cookies, now implemented in all modern browsers, for CSRF protection. That would be in keeping with the theme of being built on modern web fundamentals.

@davidbarratt-drizly
Copy link

davidbarratt-drizly commented Jan 9, 2022

Perhaps Remix is relying on the SameSite feature of cookies, now implemented in all modern browsers, for CSRF protection. That would be in keeping with the theme of being built on modern web fundamentals.

If that's the case, then it should be made explicit in the documentation, but in my example above, the SameSite attribute is irrelevant if it's not setting/validating a CSRF cookie in the first place.

@rphlmr
Copy link
Contributor

rphlmr commented Jan 24, 2022

@ngbrown
Copy link
Contributor

ngbrown commented Mar 12, 2022

If using OAuth to start an authenticated session, one often has to use SameSite=none to get everything to work because of the redirect flow. Which prevents using the same cookies for CSRF in forms...

@machour machour added the docs label Mar 24, 2022
@remix-run remix-run locked and limited conversation to collaborators Apr 19, 2022
@chaance chaance converted this issue into discussion #2906 Apr 19, 2022

This issue was moved to a discussion.

You can continue the conversation there. Go to discussion →

Labels
Projects
None yet
Development

No branches or pull requests

6 participants