Fork me on GitHub
Subscribe 4

Ticket #895 (fixed bug)

random_pass() might generate URL-unfriendly passwords

  • Created: 2013-09-01 18:00:39
  • Reported by: Pierre
  • Assigned to: Franz
  • Milestone: 1.5.5
  • Component: code
  • Priority: normal

We got reports about non-working password reset links users got via mails. These had characters like + in them which have a special meaning in URLs (they are translated to spaces unless urlencoded).

The problem seems to be in random_pass which calls random_key that base64_encodes a random byte series. base64 also includes + and /.

I suggest to not use base64 and only allow a-z0-9 charactes here or use urlencode on the key.

The same might apply to registration and other mails; I did not check that.

History

Studio384 2013-09-01 18:03:42

  • Component set to code.
  • Milestone set to 1.5.5.

Franz 2013-09-01 20:55:17

  • Owner set to Franz.

Good catch, cheers!

Franz 2013-09-05 23:08:32

Commit 3ca3d8d to fluxbb master

#895: Fix URL-unfriendly characters in random keys with readable option.

Franz 2013-09-05 23:13:23

  • Status changed from open to fixed.

Koos 2013-09-23 16:57:09

Is it really necessary to make use of an external function? I rewrote the random_pass function after doing some research, and it is also very secure. Here it is:

//
// Generate a random password of length $len
// This function is partially based on the function genRandomPassword() used in Joomla CMS (http://http://www.joomla.org).
//
function random_pass($len)
{
	// Generate cryptographic-strength random bytes
	$num_bytes = $len + 1;
	$random_bytes = '';
	if (function_exists('openssl_random_pseudo_bytes') && (version_compare(PHP_VERSION, '5.3.4', '>=') || substr(PHP_OS, 0, 3) !== 'WIN'))
	{
		$SSLstr = openssl_random_pseudo_bytes($num_bytes, $strong);

		if ($strong)
			$random_bytes = $SSLstr;
	}

	// For unix/linux platforms
	if (strlen($random_bytes) < $num_bytes && @is_readable('/dev/urandom') && ($fh = @fopen('/dev/urandom', 'rb')))
	{
		$random_bytes = fread($fh, $num_bytes);
		fclose($fh);
	}

	// Falling back to using a pseudo-random state to generate randomness
	if (strlen($random_bytes) < $num_bytes)
	{
		$random_bytes = '';
		$random_state = microtime().uniqid(rand(), true);
		// Further initialize with the somewhat random PHP process ID
		if (function_exists('getmypid'))
			$random_state .= getmypid();

		for ($i = 0; $i < $num_bytes; $i += 16)
		{
			$random_state = md5(microtime().$random_state);
			$random_bytes .= pack('H*', md5($random_state));
		}
	}

	$salt = 'ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789';
	$base = strlen($salt);
	$password = '';

	// Convert the cryptographic-strength random bytes to a string with the numeric
	// base of the salt. Shift the base conversion on each character so the character
	// distribution is even, and randomize the start shift so it's not predictable.
	$shift = ord($random_bytes[0]);
	for ($i = 1; $i <= $len; ++$i)
	{
		$password .= $salt[($shift + ord($random_bytes[$i])) % $base];
		$shift += ord($random_bytes[$i]);
	}

	return $password;
}

Franz 2013-09-25 08:04:11

I used that particular library, because it was written by an external security researcher.