sidebase / nuxt-session

Nuxt session middleware to get a persistent session per app user, e.g., to store data across multiple requests. The nuxt session module provides the useSession() composable out of the box and sets up API endpoints to interact with your session to make working with sessions feel like a breeze.
https://sidebase.io/nuxt-session/
MIT License
189 stars 19 forks source link

Remove session-jacking vulnerability #15

Closed Voltra closed 1 year ago

Voltra commented 1 year ago

Describe the feature

The current session system models very closely the base PHP session which is to say a map of SESSION_ID -> SESSION_DATA. This, as noted in the documentation of the package, is a quite the vulnerability.

One of the ways to counter that is to change the mapping to be (SESSION_ID, USER_IP) -> SESSION_DATA. This allows:

However, this requires two things:

Moreover this brings out a number of issues:

This is not a perfect solution, it is right now just a proposition and I think people can add other solutions to expand our horizons. Something akin to Laravel's session driver might be a good thing too.

I'll try and figure out how to get the IP-restricted session system working and make a proper PR, unless someone beats me to it.

Additional information

No response

Voltra commented 1 year ago

This is what the logic should be like

graph TD;
A[Request]-->B{Has session ID?}
B-- No -->C[Create session ID]
B-- Yes -->D2{Has session expired?}
D2-- No -->D[Get IP for session ID]
D2-- Yes -->H
D-->E[Get IP from request]
E-->F{request IP matches session IP?}
F-- No -->C2[Report session-jacking attempt]
C2-->C
F-- Yes -->G[Fetch session data]
C-->H[Create empty session data]
G-->I[Make session available via middleware & composables]
H-->I
I-->J[Proceed with the request]
valiafetisov commented 1 year ago

Hey @Voltra, thanks for the proposal and the diagram! I think the concept of pining sessions to user IPs is a valid one, although, as you pointed out, might not be suitable for most users and would probably be optional due to:

If you still want to proceed, I suggest to introduce a new configuration flag, eg ipPinning: false to the default config 🙂

Atinux commented 1 year ago

Another solution would be to leverage the user agent since the browser/device is less subject to change compared to the IP (Mobile when having wifi disconnected and fallback to mobile network)

Atinux commented 1 year ago

Also, have you seen https://github.com/vvo/iron-session ?

Voltra commented 1 year ago

Accurately pinning a user is a complicated subject (often studied by companies for unethical ways of pinpoint tracking). UA might not change, but it's more likely to be similar (if not equal) to another user's. It also makes us go back to the main point, which would then be UA-jacking.

Voltra commented 1 year ago

As for iron-session, I think I've used it on a project (might just have tried it though). I think what you're getting at is making the session "travel" alongside the request, which in my opinion might be a security risk waiting to happen. In that sense, a JWT driver is definitely possible, but it's beyond the scope of this issue, and comes with its own set of problems (like proper and/or remote token invalidation).

BracketJohn commented 1 year ago

It also makes us go back to the main point, which would then be UA-jacking.

I agree that it has the same vulnerability as ids: Being jacked. But: It requires additional effort and skill to jack it + then fake it correctly to get the match.

If we think about it this way, IP as another factor is vulnerable in the same way: attacks coming from the same network or BGP attacks can lead to successful IP jacking.

I see these different factors on a scale:

-> based on this my suggestion would be that we allow using different factors, using flags to opt in / opt out + set a sensible set of factors in the beginning (eg, just IDs).

Impleementing the different strategies is out of scope for this issue, but maybe we can agree to:

What do you think @Voltra?

valiafetisov commented 1 year ago

to leverage the user agent

Maybe it can also be optional check, but from my perspective it wouldn't add much protection. The main difference between IP-pinning vs UA-pinning is that UA is coming from the same network actor as session id, while IP information is coming from the server, not user. Practically it means that having a "read" access to the communication is enough to pretend the same user with UA-pinning, while with IP-pinning the attacker should also be able to do requests (or "write") from the user perspective.

valiafetisov commented 1 year ago

Also, have you seen https://github.com/vvo/iron-session ?

Thanks for the reference, although it's a bit out of topic here. I created another issue https://github.com/sidebase/nuxt-session/issues/17 to discuss this topic. You're welcome to bring counter arguments 🙂