Fork me on GitHub
Subscribe 4

Ticket #1058 (fixed bug)

hash_equals(): Expected known_string to be a string, null given

  • Created: 2015-11-12 09:25:30
  • Reported by: chris98
  • Assigned to: Oldskool
  • Milestone: 1.5.10
  • Component: authentication
  • Priority: normal

If the user username/password is incorrect when logging into the forum and the user is using PHP 5.6+, then you'll get the following warning:

Errno [2] hash_equals(): Expected user_string to be a string, null given in /home/XXX/public_html/forums/include/functions.php on line 147

This is taken from my fork, so the line number may not be exactly accurate. But it points to this line:

return hash_equals($a, $b);

To remedy this, the following fix should be made:

return hash_equals((string)$a, $b);

The reason you get the warning is because if no results are returned from the database (e.g. incorrect credentials) then the password will be NULL.

Thanks to Quy for pointing this out to me wink

History

chris98 2015-11-12 09:31:00

  • Description changed. (Diff)
  • Summary changed from hash_equals(): Expected user_string to be a string, null given to hash_equals(): Expected known_string to be a string, null given.

adaur 2015-11-12 09:33:24

  • Milestone set to 1.5.10.

I took the function from WordPress, I assumed it would be safe to use as-is.

Feel free to submit a PR smile

chris98 2015-11-12 09:47:40

Yeah, I thought so too big_smile

Should have made a PR, but you might have to check it: https://github.com/fluxbb/fluxbb/pull/171

Visman 2015-11-12 09:49:16

Return true or false?
If true, then it is security problem.

Visman 2015-11-12 09:52:32

And why not add a condition

function pun_hash_equals($a, $b)
{
  if (!is_string($a) || !is_string($b))
    return false;

adaur 2015-11-12 09:56:08

Then if you submit two nulls, it will return false instead of true, right?

chris98 2015-11-12 09:58:08

That's kind of still vulnerable to a timing attack though - the whole point of this is to prevent a timing attack but if you're just checking if it's a string and then returning back then it doesn't do the job leaving PHP versions 5.6+ vulnerable still if the user enters an incorrect password.

adaur 2015-11-12 09:59:11

+1 chris. If we exit the function before the call to hash_equals or the fallback, we're still prone to attacks.

Let's do two typecasts

Visman 2015-11-12 10:03:19

The original code

	$a_length = strlen($a);

	if ($a_length !== strlen($b))
		return false;

identically

	if (!is_string($a) || !is_string($b))
		return false;

	if (function_exists('hash_equals'))
		return hash_equals($a, $b);


	$a_length = strlen($a);

	if ($a_length !== strlen($b))
		return false;

chris98 2015-11-12 10:03:19

Done, casted both of them to a string.

adaur 2015-11-12 10:09:09

  • Owner set to Oldskool.
  • Status changed from open to fixed.

adaur 2015-11-12 10:30:50

I thought oldskool merged it but he did it on the wrong branch wink

All changes need to go on 1.5-next

adaur 2015-11-13 19:05:39

Well, it should. I don't have the rights to do the merges by myself.

Franz 2015-11-13 22:48:06

I'll take care of it.

quy 2016-01-09 22:29:24

Commit 1911972 to fluxbb 1.5-next

#1058 Remove placement reference in BBCode info text

Franz 2016-01-11 06:45:10

Commit 9b820ec to fluxbb 1.5-next

Merge pull request #186 from Quy/1058-bbcode

#1058 Remove placement reference in BBCode info text

quy 2016-06-16 07:52:24

Commit 7546b62 to fluxbb master

#1058 Remove placement reference in BBCode info text