Remove session-jacking vulnerability #15
Comments
|
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]
|
|
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 |
|
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) |
|
Also, have you seen https://github.com/vvo/iron-session ? |
|
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. |
|
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). |
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? |
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. |
Thanks for the reference, although it's a bit out of topic here. I created another issue #17 to discuss this topic. You're welcome to bring counter arguments 🙂 |
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
The text was updated successfully, but these errors were encountered: