Skip to content
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

Possible unserialize() error in CWebUser #6

Closed
qiangxue opened this issue Feb 15, 2012 · 0 comments
Closed

Possible unserialize() error in CWebUser #6

qiangxue opened this issue Feb 15, 2012 · 0 comments

Comments

@qiangxue
Copy link
Member

What steps will reproduce the problem?

  1. Switch on $enableCookieValidation of the request component.
  2. Switch on $allowAutoLogin of the user component.
  3. Login a user and make sure a cookie is set ($duration > 0)
  4. Disable $enableCookieValidation in the config.
  5. Reload the page and make sure something like
    Yii::app()->user->getIsGuest() gets called.

What is the expected output?

Yii::app()->user->getIsGuest() should either return true or false.


What do you see instead?

PHP Error
Beschreibung

unserialize() [<a href='function.unserialize'>function.unserialize</a>]:
Error at offset 0 of 89 bytes
Quelldatei

Y:\xampplite\htdocs\Example.com\yii\framework\web\auth\CWebUser.php(323)

00311: * Populates the current user object with the information
obtained from cookie.
00312: * This method is used when automatic login ({@link
allowAutoLogin}) is enabled.
00313: * The user identity information is recovered from cookie.
00314: * Sufficient security measures are used to prevent cookie data
from being tampered.
00315: * @see saveToCookie
00316: */
00317: protected function restoreFromCookie()
00318: {
00319: $app=Yii::app();
00320:
$cookie=$app->getRequest()->getCookies()->itemAt($this->getStateKeyPrefix());
00321: if($cookie && !empty($cookie->value) &&
($data=$app->getSecurityManager()->validateData($cookie->value))!==false)
00322: {
00323: $data=unserialize($data);
00324: if(isset($data[0],$data[1],$data[2],$data[3]))
00325: {
00326: list($id,$name,$duration,$states)=$data;
00327: $this->changeIdentity($id,$name,$states);
00328: if($this->autoRenewCookie)
00329: {
00330: $cookie->expire=time()+$duration;
00331:
$app->getRequest()->getCookies()->add($cookie->name,$cookie);
00332: }
00333: }
00334: }
00335: }


What version of the product are you using? On what operating system?

Using Latest from trunk on Windows/PHP 5.3


Since a cookie is stored client-side and may be valid for a long time, I
suggest to change the above error-line to:

$data=@unserialize($data);

Otherwise a user may get an error when $enableCookieValidation property
gets changed in a production environment.

Migrated from http://code.google.com/p/yii/issues/detail?id=776


earlier comments

a.jilkin said, at 2010-01-01T18:50:30.000Z:

using of @ is not good idea, because it can cause problems in future w/o any output

keyboard.idol said, at 2010-01-01T23:49:24.000Z:

Normally I don't use @ at all, but in this case I see no other solution. What you mean with "problems in future w/o any output"?

keyboard.idol said, at 2010-01-11T17:06:12.000Z:

Also take a look at this: http://www.suspekt.org/2009/12/09/advisory-032009-piwik-cookie-unserialize-vulnerability/

As far as I can tell there is no Yii class that is vulnerable. Still, 3rd party
extension could be. Look how Piwik was fixed: http://dev.piwik.org/trac/changeset/1637

I think it would be good to apply such a preg_match to all core components in order
to make sure no objects are "created" by an unserialized cookie. Or optional
implement it into CHttpRequest/CSecurityManager in some way so that all cookies are safe?

a.jilkin said, at 2010-01-17T21:04:47.000Z:

"problems in future w/o any output" -- I mean, that using of @ will block all errors, warnings and notices

also I know, that unserialize() has a problem with UTF strings
(http://www.php.net/manual/en/function.unserialize.php#93606)

qiang.xue said, at 2010-02-20T03:17:22.000Z:

This issue was closed by revision r1831.

@vaijanath vaijanath mentioned this issue Oct 9, 2013
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant