search

mozilla / node-client-sessions

secure sessions stored in cookies
Mozilla Public License 2.0
759 stars 104 forks source link

Renaming cookies could lead to security problem #111

Closed q42jaap closed 8 years ago

q42jaap commented 8 years ago

Hi,

I have an application where I have two login cookies. Whether this is a good idea or not, client-sessions supports this scenario. I have a cookie for "normal" users and a cookie for "admin" users. I'd like the admin users to simulate that they are a normal user without logging the admin out. There are several ways to do this, but I found having two cookies makes my code a bit better then a isAdmin:true, originalUserId:xxx solution.

The encryption and signing of the cookie doesn't take the cookie name into account.

The problem lies in a subtle thing a user that is able to login can do: He can copy the contents of a cookie into another cookie. Of course it's smarter to have different encryption keys for this, but client-session doesn't warn against this.

I would suggest that the signature should be extended with the cookieName (it's not in the docs anyway).

A workaround for this could be to use different secret (or encryptionKey+signatureKey). I'm currently just checking a value in the cookie that I've provided on login.

seanmonstar commented 8 years ago

This isn't an issue for client-sessions. The name is encoded into the cookie value: https://github.com/mozilla/node-client-sessions/blob/master/lib/client-sessions.js#L219

And when decoding, if the name encoded in the value doesn't match the cookie name, the cookie is invalid: https://github.com/mozilla/node-client-sessions/blob/master/lib/client-sessions.js#L324

seanmonstar commented 8 years ago

And corresponding tests that it works: https://github.com/mozilla/node-client-sessions/blob/master/test/all-test.js#L980-L1026