yiisoft / yii2

Yii 2: The Fast, Secure and Professional PHP Framework
http://www.yiiframework.com
BSD 3-Clause "New" or "Revised" License
14.23k stars 6.91k forks source link

Cookie Reuse attack vulnerability #8537

Closed alialamine closed 3 years ago

alialamine commented 9 years ago

Hello, Apparently yii2's session manager is vulnerable to Cookie Reuse attacks (type of session hijacking).

Steps to repeat:

I tried this with pasting the cookies on a different computer in the same network and it worked (didn't try for another ip though)

Desired effect: When I logout the cookies saved should not work

Note: I only tried this with the default PHP session manager. Didn't try the db or cache ones.

klimov-paul commented 9 years ago

Are you sure yii\web\User::enableAutoLogin is disabled in your case?

alialamine commented 9 years ago

@klimov-paul Yes I am sure. And I did say I pressed the Logout button then pasted the cookies and found myself logged in again. I also pasted the cookies on another computer after logging out and I managed to login without using the password (which should never happen)

cebe commented 9 years ago

I am unable to reproduce this with a plain install of the basic application. What are your settings for the application components user and session?

alialamine commented 9 years ago

1) Sorry just checked again, I was indeed using the remember me. 2) I'm using the advanced installation (didn't check the basic). And just did a clean install and it works.

As for the remember me, even if I'm using it, the cookies should be unvalid when I logout.

klimov-paul commented 9 years ago

As for the remember me, even if I'm using it, the cookies should be unvalid when I logout.

Although this can be achieved, it whould not improve the security for this case. If 'remember me' cookie is stolen and pasted into other browser, attacker will have a new session for logged in user, even if previous one has not been ended yet (logout is not triggered). Even if 'remember me' cookie value will be banned somehow after logout, it will be already too late.

AutoLogin feature ('remember me') is unsafe by its definition, that is why we always place a checkbox for its activation, allowing user to choose, if he willing to take the involved risks or not.

samdark commented 9 years ago

Both user browser and channel are considered safe. If they are not there are endless possibilities to get user credentials.

kidol commented 9 years ago

The problem is that the content of an autologin cookie is valid forever. A session on the other hand will expire server-side. As far as I can tell, currently there is no check whether the content of an autologin-cookie expired. This could be done (by storing some timestamp in cookie) and it would be safe due to hmac. Means if autologin-cookie is valid for 7 days and attacker tries to login with that cookie data at day 8, he would fail. Currently he would have success at day 9999.

@klimov-paul As I see it, this is not about real-time like attack, but about an attack some time in the future. However, if user explicitly logs out, why wouldn't it be good practice to invalidate all related auth data? The advanced app's user model already has generateAuthKey() method. If called at logout + $user->update() then the autologin-cookie data should be invalid, right?

@samdark User browser might be safe at the time of login, but not in future (theft, trojan, whatever). Channel is never safe on HTTP protocol. If site upgrades to HTTPS later, previously captured autologin-cookies still work forever. Unlikely but: If a user does not use or the site does not support forward secrecy, then the captured https traffic could be decrypted at a later time.

In cryptography, forward secrecy (FS; also known as perfect forward secrecy, or PFS[1]) is a property of key-agreement protocols ensuring that a session key derived from a set of long-term keys cannot be compromised if one of the long-term keys is compromised in the future.

klimov-paul commented 9 years ago

Means if autologin-cookie is valid for 7 days and attacker tries to login with that cookie data at day 8, he would fail. Currently he would have success at day 9999.

You may specify any lifetime duration for auto login cookie as you like at User::switchIdentity()

However, if user explicitly logs out, why wouldn't it be good practice to invalidate all related auth data?

Auto login cookie are removed after user logs out.

kidol commented 9 years ago

You may specify any lifetime duration for auto login cookie

The lifetime only affects the cookie in the users browser. If I manage to somehow take the cookie data out from his browser, or if I can capture it, it's valid forever and I can re-use it later. So if I set cookie lifetime in my app to 1 day or 7 days, it doesn't matter. Data itself is valid forever.

Auto login cookie are removed after user logs out.

Data itself is still valid for login as I understand, because auth key did not change.

klimov-paul commented 9 years ago

The lifetime only affects the cookie in the users browser. If I manage to somehow take the cookie data out from his browser, or if I can capture it, it's valid forever and I can re-use it later.

As I have already said here: https://github.com/yiisoft/yii2/issues/8537#issuecomment-105055669 we can make auto login cookie value being invalidated on server side. BUT It will NOT close a security breach here, because if this cookie is used after being stolen, but before user logout, they WILL grant access to the user account anyway. You are asking for creation complex server-side mechanism, which can prevent identity theft ONLY, if attacker is nice enough to wait until user actually logs out, before attempt ot use his auto login cookie. This simply does not worth the effots.

kidol commented 9 years ago

complex

Not complex at all. Timestamp can be stored in cookie. Duration itself is already stored for renewing the cookie. And invalidating by generating new authkey on explicit logout is also not complex.

which can prevent identity theft ONLY, if attacker is nice enough to wait until user actually logs out

No. Assume this:

  1. User logs in, cookie duration set to 7 days
  2. At the same time, attacker captures cookie
  3. User is normally visiting site every day
  4. On day 8, the attacker tries to login with the captured cookie
  5. It will work

It would NOT work if there is timestamp within the cookie data. Because if the internal duration is limited to 7 days for the stolen cookie, he can not use it at day 8. For normal user nothing would change because original cookie gets updated automatically every day he visits.

Again, problem is the cookie data itself is valid forever.

samdark commented 9 years ago

Got it. Makes sense.

alialamine commented 9 years ago

@kidol Using timestamps saved in cookies will not make any difference since the cookies can be tampered with. As for recreating the session, that will log out the user from all his devices, which is not what the user wants.

I can think of an implementation on the db-session where the session itself have a flag that determines if it is permanent or not, and use the session id as the token in the cookies (so that it don't get deleted by the browser when closing it). That way when the user logs out from that session, the flag is unset hense it isn't a remember-me token anymore.

No idea about the other session types though.

kidol commented 9 years ago

@ali-alamine No it can not be tampered, because cookies are hashed with hmac. You can still copy a cookie and reuse it, however since it's not possible to modify the baked-in timestamp, an attacker is limited to it. https://github.com/yiisoft/yii2/blob/master/framework/web/Request.php#L1218

As for recreating the session, that will log out the user from all his devices, which is not what the user wants.

Makes sense, but if you want perfect control over sessions (a "logout this/all sessions" feature for the users, ability to abort a session as an admin, etc.), I think it's best to implement it on your own. Using DbSession is the first step. You can modify it to add other stuff to the session like ip range, user-agent etc for additional security.

CyberPunkCodes commented 9 years ago

The cookie should be bound to the IP Address and additionally the User Agent. A system I did a while back (before I used Yii) encrypted the IP and UA combined. In the script, I decoded the string and checked that the IP and UA matched, before letting them in. I am not saying it is "the" solution. I think at the very least, a cookie should be bound to the user via some strong identifying way, like their IP.

samdark commented 9 years ago

IP — probably not because of static IPs aren't common among ISPs in many countries. User Agent may change with browser upgrade.

cbhp commented 9 years ago

The Cookie should neither be bound to the IP address nor to the UA. For example, I use Yii for a webapp. The user wants to stay logged in when he leaves home (WLAN / ISP sets new IP every day) and is outdoor (mobile connection) with his tablet, and even if there's an update for his browser (every few weeks a new UA for Firefox). If an attacker has access to the client computer, there are so many ways for stealing the credentials. Session riding is one of the smallest issues. For in-browser security, just prevent XSS, set httpOnly = true and use TLS.

WouterSlob commented 9 years ago

@cbhp You are right. Also don't forget to set the "secure" property on your cookie to ensure it is only send over a secure connection. This can easily be configured in the config of the app:

'user' => [ ... 'identityCookie' => [ 'name' => '_identity', 'httpOnly' => true, 'secure' => true, ], ], 'session' => [ 'cookieParams' => [ 'httpOnly' => true, 'secure' => true, ], ],

Depending on the type of application you can always ask the user to re-enter his password before allowing critical actions (eg changing personal information, buying stuff on a webshop).

cbhp commented 8 years ago

Three more reasons why not to bind a session to the user agent:

(taken from https://en.wikipedia.org/wiki/Session_fixation#User_Agent)

Last bot not least, user agent is not a security feature. (User/Hacker can manipulate/copy it.)

hugodiniz commented 8 years ago

Using a identity class implemented as a Active Record with a expiration property, and checking the expiration in validateAuthKey method is not enough ? I agree is not good to leave the expiration control only in the client side. I think the framework has not to change in this point. But the doc guide is not explaining the consequences of enableAutoLogin, in my opinion.

scottix commented 8 years ago

I would agree we would need some server side expiration and to possibly further the conversation.

I think having a pre-made auth token system would be beneficial for users. Similar to what you have done with RBAC.

You would need an additional table or column depending on what you use, to manage all the logins from your phone, tablet, work computer, etc... Google and Facebook do this by letting me know where I am logged in and be able to revoke those accesses. Also each one of those endpoints could have a different auth token.

You can do this today but having a pre-made solution would be nice with a few helper functions.

ghost commented 8 years ago

I have been working on an extension to replace the standard "Remember Me" function. I believe this will solve many of the issues the people have brought up here.

The extension is here: maine-mike/yii2-remember-me

For this extension to work, this pull request has to be integrated. I'm hoping it will be included in 2.0.9.

https://github.com/yiisoft/yii2/pull/11558

krukru commented 7 years ago

So is there any progress on the issue? Does this mean that there is still no built-in way to invalidate a stolen cookie?

samdark commented 7 years ago

@krukru the way to invalidate such a cookie is to regenerate auth key.

CyberPunkCodes commented 7 years ago

@samdark - I think the biggest issue they are having, is when to logically reset the auth key. Yes, the only way to solve the re-use attack vuln is to reset the auth key.

The way Yii2 handles the auth key, is that it is created during signup and never ever ever changed. Which, if someone snags your cookie, they can always login to your account.

You can reset the auth key on every valid login. Down side, it will log you out of other devices or logout shared users.

You can reset the auth key after the user changes their password. Downside, it will log you out and logout other shared users. You could have your reset-password process rebuild the cookie to not log you out, but that won't spare the shared users.

Maybe you could store a 'authkey_resetat' column to store the timestamp it was last changed. Maybe if it has been more than 30 days, and the user logs in via the login form, it resets it. This would spare that user from losing a valid cookie (say they are on day 15 of a 30 day cookie). Downside, it kills shared users/devices once reset. There is no way getting around that tbh.

I think that would be the best solution. Store the last reset time, and change the auth key on login.

samdark commented 7 years ago

@WadeShuler all solutions you've mentioned are degrading usability and I think in some cases it's more important than extra security.

CyberPunkCodes commented 7 years ago

@samdark I agree. Out of the box it shouldn't be implemented.

Maybe an option to enable some sort of method would be a nice enhancement. I could create a PR for my method: store last authkey reset time stamp in db, 60 day max life, and check/reset on login. All optional configs.

Or I could create a module on my own, reference it in the docs, and see if the community has an interest.

Aside from that: is there a better way to handle cookies than Yii currently does? Even if it requires redoing the entire cookie handling.. just curious

samdark commented 7 years ago

I think we'd better document it.

samdark commented 3 years ago
  1. Force-invalidating sessions/cookies on logout isn't a good idea since it is a usability nightmare when having mutliple devices such as phone and computer at work where you log in and log out daily.
  2. Next release will include change that will invalidate cookies and existing session on authKey change so force-logout could be easily implemented in apps if needed.
  3. Force-checking auto-login cookie expiry is a great idea and we'll implement that in Yii 3. It seems it's impossible to do so without HMAC-signing cookies and we don't want to always force signing in Yii 2. Meanwhile, invalidating authKey could be used from time to time to enhance security.
schmunk42 commented 3 years ago
  • Next release will include change that will invalidate cookies and existing session on authKey change so force-logout could be easily implemented in apps if needed.

I assume this does not break BC, when I do not change my code/config, right?

samdark commented 3 years ago

Well, it depends. Your application will break if you'll throw exception from validateAuthKey() or getAuthKey() and won't use "remember me" cookie. If these are just left unimplemented (returning true and null), then no issue, it will work as before.