Skip to content

Commit

Permalink
Made generateNonce() return value url safe
Browse files Browse the repository at this point in the history
This change is primarly to make sure we do not have to url encode the xsrf token.

Also, the default value for the method is now 30 instead of 20. (the magic 20 number was undocumented). The magic number used in the for loop for the fallback is also replace by the length parameter value: the sorter the nonce, the more iteration we have to make to shuffle things more.

Finally the method will now throw an Exception if the length is smaller than 1.

Fixes #2567
  • Loading branch information
nitriques committed Mar 3, 2016
1 parent b35eba8 commit 34a3b50
Showing 1 changed file with 34 additions and 17 deletions.
51 changes: 34 additions & 17 deletions symphony/lib/toolkit/class.xsrf.php
Original file line number Diff line number Diff line change
Expand Up @@ -61,33 +61,50 @@ public static function removeSessionToken($token = null)
* falling back to using `/dev/urandom` and a microtime implementation
* otherwise
*
* @param integer $length
* @param integer $length optional. By default, 30.
* @return string
* base64 encoded, url safe
*/
public static function generateNonce($length = 20)
public static function generateNonce($length = 30)
{
// Base64 encode some random binary data, and strip the "=" if there are any.
if (function_exists("openssl_random_pseudo_bytes")) {
return str_replace("=", "", base64_encode(openssl_random_pseudo_bytes($length)));
$random = null;
if ($length < 1) {
throw new Exception('$length must be greater than 0');
}

// Fallback if openssl not available
if (is_readable("/dev/urandom")) {
if (($handle = @fopen("/dev/urandom", "rb")) !== false) {
$bytes = fread($handle, $length);
fclose($handle);
return str_replace("=", "", base64_encode($bytes));
// Get some random binary data from open ssl, if available
if (function_exists('openssl_random_pseudo_bytes')) {
$random = openssl_random_pseudo_bytes($length);
}

// Fallback to /dev/urandom
else if (is_readable('/dev/urandom')) {
if (($handle = @fopen('/dev/urandom', 'rb')) !== false) {
$random = @fread($handle, $length);
@fclose($handle);
}
}

// Fallback if /dev/urandom not readable.
$state = microtime();
// Fallback if no random bytes were found
if (!$random) {
$random = microtime();

for ($i = 0; $i < 1000; $i += 20) {
$state = sha1(microtime() . $state);
for ($i = 0; $i < 1000; $i += $length) {
$random = sha1(microtime() . $random);
}
}

return str_replace("=", "", base64_encode(substr($state, 0, $length)));
// Convert to base64
$random = base64_encode($random);

// Replace unsafe chars
$random = strtr($random, '+/', '-_');
$random = str_replace('=', '', $random);

// Truncate the string to specified lengh
$random = substr($random, 0, $length);

return $random;
}

/**
Expand All @@ -114,7 +131,7 @@ public static function getToken()
{
$token = self::getSessionToken();
if (is_null($token)) {
$nonce = self::generateNonce(20);
$nonce = self::generateNonce();
self::setSessionToken($nonce);

// Handle old tokens (< 2.6.0)
Expand Down

2 comments on commit 34a3b50

@brendo
Copy link
Member

@brendo brendo commented on 34a3b50 Mar 3, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me

@nitriques
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks mate!

Please sign in to comment.