Skip to content
This repository has been archived by the owner on Apr 8, 2023. It is now read-only.

Login is Vulnerable to PHP Type Juggling #78

Open
reznok opened this issue Jul 30, 2018 · 5 comments
Open

Login is Vulnerable to PHP Type Juggling #78

reznok opened this issue Jul 30, 2018 · 5 comments
Labels

Comments

@reznok
Copy link

reznok commented Jul 30, 2018

The following code in User.php is vulnerable to PHP type juggling and timing attacks:

if (($user) && ($user['password'] == $password) && ($user['locked'] == 0) && ($user['authused'] == 0))
{
        return $user;
}

Because "==" is being used instead of hash_equals(), the following passwords, when compared with their md5 hashes, would be all registered as equal in the auth code:
(Format is "password:md5(password))

240610708:0e462097431906509019562988736854
QNKCDZO:0e830400451993494058024219903391
aabg7XSs:0e087386482136013740957780965295

Because the hashes are in format: 0e[0-9], PHP will treat the string as an integer when doing loose comparisons. In that notation, they all evaluate as being int(0), causing them to be equal.

php > echo md5("QNKCDZO");
0e830400451993494058024219903391
php > echo md5("240610708");
0e462097431906509019562988736854

php > var_dump("0e830400451993494058024219903391" == "0e462097431906509019562988736854");
bool(true)

php > var_dump(hash_equals("0e830400451993494058024219903391", "0e462097431906509019562988736854"));
bool(false)

($user['password'] == $password)

Should be changed to:
hash_equals($user['password'], $password)

More Type Juggling Information:
https://www.owasp.org/images/6/6b/PHPMagicTricks-TypeJuggling.pdf

Note:
This is a low-ish severity issue, (requires someone to have a password that md5's to that exact format), just something I figured should be pointed out.

Thanks to @mjrider for correcting my original solution.

@mjrider
Copy link

mjrider commented Jul 31, 2018

Please, do no use === but hash_equals — Timing attack safe string comparison

@brammittendorff-dd
Copy link

@mjrider is absolutely right

@reznok
Copy link
Author

reznok commented Jul 31, 2018

Good catch @mjrider, I'm updating my issue.

@jfm-so
Copy link
Owner

jfm-so commented Aug 2, 2018

Thank you for the contribution, added to the todo list.

@CryptorClub
Copy link

@mjrider or @reznok hello, why do not make pull request with modifications?

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

No branches or pull requests

5 participants