Permalink
Browse files

Fix #311: non cryptographically secure CSRF tokens

Neither `uniqid()` nor `rand()` are sufficent for cryptographic
purposes, and combining them does not really improve the situation.
Applying `md5()` on the resulting value may even weaken the token
further.

Instead we use 128bits of entropy as recommended by both OWASP and CWE
for session identifiers which are rather comparable to CSRF tokens in
this regard.  We obtain the values via `random_bytes()`.
  • Loading branch information...
cmb69 committed Oct 7, 2017
1 parent 9fa061e commit 619543cb2acfda62ba2a87409996ac194949125e
Showing with 1 addition and 3 deletions.
  1. +1 −3 cmsimple/classes/CSRFProtection.php
@@ -63,13 +63,11 @@ public function __construct($keyName = 'xh_csrf_token', $perRequest = false)
* for inclusion in an HTML form.
*
* @return string HTML
*
* @todo Use cryptographically stronger token?
*/
public function tokenInput()
{
if (!isset($this->token)) {
$this->token = md5(uniqid(rand()));
$this->token = bin2hex(random_bytes(16));
}
$o = '<input type="hidden" name="' . $this->keyName . '" value="'
. $this->token . '">';

0 comments on commit 619543c

Please sign in to comment.