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

Magic Hash Vulnerability #8326

Closed
paragonie-scott opened this issue Nov 8, 2015 · 3 comments
Closed

Magic Hash Vulnerability #8326

paragonie-scott opened this issue Nov 8, 2015 · 3 comments

Comments

@paragonie-scott
Copy link
Contributor

return md5($password . substr($hash, 33)) == substr($hash, 0, 32);

You're using the non-strict equality operator to compare hashes...

http://blog.astrumfutura.com/2015/05/phps-magic-hash-vulnerability-or-beware-of-type-juggling/

cc @padraic @ircmaxell @enygma

@mbabker mbabker closed this as completed in aa7d0ac Nov 9, 2015
wilsonge added a commit that referenced this issue Nov 9, 2015
Use strict verification of legacy Joomla MD5 hash (Fix #8326)
@paragonie-scott
Copy link
Contributor Author

Another suggestion: use a timing safe comparison.

(Also: make this use hash_equals() in PHP 5.6+.)

@mbabker
Copy link
Contributor

mbabker commented Nov 9, 2015

There was a pull request for that at one point (#4206 then #7754) but in pretty typical manner around these parts it was over the head of most folks testing/reviewing patches and ended up abandoned/closed.

@beat
Copy link
Contributor

beat commented Nov 14, 2015

👍 From code review standpoint, the referred commit as well as follow-up commits seem to address this issue properly.

Btw, Joomla upgrades the stored password hash at each login if needed. Thus, this issue can only occur on a very old account that has not been logged in for a very long time, or on a very old unmaintained Joomla installation (one of these has to be older than around 3 years so that the password hasn't been upgraded at a login with a recent Joomla version yet for that compare case to occur).

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

No branches or pull requests

4 participants