Fixed \XF\Util\Random::getRandomString() seems suspectible to timing attacks

Kirby

Well-known member
Affected version
2.2.13
\XF\Util\Random::getRandomString() generates a cryptographically secure random value (by calling \XF\Util\Random::getRandomBytes()) but then calls base64_encode to generate a string from those bytes.

AFAIK, base64_encode is not constant time so could be vulnerable to timing attacks.

It might therefore be better to use sodium_bin2base64 instead (if available) or fallback to bundled ParagonIE_Sodium_Core_Base64_UrlSafe if not.
 
While it doesn't hurt to use something better than base64_encode, realistically someone would need local access to the server to make the necessary timing measurements... at which point the attacker doesn't need to try to reverse engineer the random string, they can just dump the database or look at network streams. Or simply look at the output of the getRandomString function... no timing attack needed.

But like I said, it doesn't hurt other than dealing with the fact that not all PHP installations have sodium compiled in. Might be simpler from a support standpoint to just wrap base64_encode in your own method that factors in timing.
 
But like I said, it doesn't hurt other than dealing with the fact that not all PHP installations have sodium compiled in. Might be simpler from a support standpoint to just wrap base64_encode in your own method that factors in timing.
XenForo bundles paragonie/sodium_compat which does provide a pure PHP implementation of constant-time base64 (as already pointed out in the first post).
 
Thank you for reporting this issue, it has now been resolved. We are aiming to include any changes that have been made in a future XF release (2.2.14).

Change log:
Use sodium_bin2base64 over base64_encode when generating random strings.
There may be a delay before changes are rolled out to the XenForo Community.
 
Back
Top Bottom