Passwords: possible NULL byte vulnerability

[JanTvrdik]

Just a note, someone should read carefully http://blog.ircmaxell.com/2015/03/security-issue-combining-bcrypt-with.html and verify whether and how it affects Passwords class.

[dg]

Not affects.

[JanTvrdik]

You are wrong. It DOES affect Passwords:

$hash = Passwords::hash("\x00 foo");

dump(Passwords::verify("\x00 foo", $hash)); // TRUE, expected TRUE
dump(Passwords::verify("\x00 bar", $hash)); // TRUE, expected FALSE

Please reopen this, this has to be resolved.

[JanTvrdik]

Possible workarounds:

• disallow NULL byte (preferred)
• silently replace NULL byte with "\x01" (dangerous, we should not invent our crypto)
• base64 / md5 all inputs (solves the issue but break compatibility)

[hrach]

Also, if I understand this class is compatible to native functions. So this compatibility should be preserved.

[dg]

It is not possible to break compatibility, so Passwords::verify($pass, $hash) must always work. This weakness is similar to one from PHP < 5.3.7, so it will be probably fixed by updating $2y$ to $2z$. When it will be confirmed, Passwords::needsRehash() should be updated (thus I reopened it). But it is bad idea to find our solution and complicate situation.

NULL char in password is possible when somebody rehashes MD5/SHA-1 hashes stored as binary strings, it is not so unusual scenario, but now is ball on the PHP side.

[fprochazka]

Isn't \x00 null byte control character, that would have been stripped away by the request factory?

[dg]

@fprochazka https://github.com/nette/security/issues/14#issuecomment-80353247

[JanTvrdik]

@fprochazka Yes, but (generally speaking) you don't know anything about what values user pass to Password.

[spaze]

The passwords class in Nette 3/master now only wraps native functions so all "upstream" changes to needsRehash and others will be automagically used by Nette too.

Maybe this can be closed now.