Opening several pages with forms invalidate security tokens

Phalcon is storing the current security token in the session element $PHALCON/CSRF/KEY$. You can extend the Security class to support individual security tokens for each form:

<?php
/**
 * Prefixed Security
 * 
 * @author Wenzel Pünter <[email protected]>
*/
class PrefixedSecurity extends \Phalcon\Security
{
    /**
     * Alphanumerical Filter
     * 
     * @param string $value
     * @return string
    */
    private static function filterAlnum($value)
    {
        $filtered = '';
        $value = (string)$value;
        $value_l = strlen($value);
        $zero_char = chr(0);

        for($i = 0; $i < $value_l; ++$i) {
            if($value[$i] == $zero_char) {
                break;
            }

            if(ctype_alnum($value[$i]) === true) {
                $filtered .= $value[$i];
            }
        }

        return $filtered;
    }

    /**
     * Generates a pseudo random token key to be used as input's name in a CSRF check
     *
     * @param string $prefix
     * @param int|null $numberBytes
     * @return string
     * @throws \Phalcon\Security\Exception
     */
    public function getTokenKey($prefix = '', $numberBytes = null)
    {
        if(is_string($prefix) === false) {
            throw new \Phalcon\Security\Exception('Invalid prefix.');
        }

        if(is_null($numberBytes) === true) {
            $numberBytes = 12;
        } elseif(is_int($numberBytes) === false) {
            throw new \Phalcon\Security\Exception('Invalid parameter type.');
        }

        if(function_exists('openssl_random_pseudo_bytes') === false) {
            throw new \Phalcon\Security\Exception('Openssl extension must be loaded');
        }

        $random_bytes = openssl_random_pseudo_bytes($numberBytes);
        $base64bytes = base64_encode($random_bytes);
        $safe_bytes = self::filterAlnum($base64bytes);

        //@warning no length check for $safe_bytes

        if(is_object($this->_dependencyInjector) === false) {
            throw new \Phalcon\Security\Exception('A dependency injection container is required to access the \'session\' service');
        }

        $session = $this->_dependencyInjector->getShared('session');
        if(is_object($session) === false ||
            $session instanceof \Phalcon\Session\AdapterInterface === false) {
            throw new \Phalcon\Security\Exception('Session service is unavailable.');
        }

        $session->set('$PHALCON/CSRF/KEY$'.$prefix.'$', $safe_bytes);
        return $safe_bytes;
    }

    /**
     * Generates a pseudo random token value to be used as input's value in a CSRF check
     *
     * @param string $prefix
     * @param int|null $numberBytes
     * @return string
     * @throws \Phalcon\Security\Exception
     */
    public function getToken($prefix = '', $numberBytes = null)
    {
        if(is_string($prefix) === false) {
            throw new \Phalcon\Security\Exception('Invalid prefix.');
        }

        if(is_null($numberBytes) === true) {
            $numberBytes = 12;
        } elseif(is_int($numberBytes) === false) {
            throw new \Phalcon\Security\Exception('Invalid parameter type.');
        }

        if(function_exists('openssl_random_pseudo_bytes') === false) {
            throw new \Phalcon\Security\Exception('Openssl extension must be loaded');
        }

        $random_bytes = openssl_random_pseudo_bytes($numberBytes);

        //@note MD5 is weak
        $token = md5($random_bytes);

        if(is_object($this->_dependencyInjector) === false) {
            throw new \Phalcon\Security\Exception('A dependency injection container is required to access the \'session\' service');
        }

        $session = $this->_dependencyInjector->getShared('session');
        if(is_object($session) === false ||
            $session instanceof \Phalcon\Session\AdapterInterface === false) {
            throw new \Phalcon\Security\Exception('Session service is unavailable.');
        }

        $session->set('$PHALCON/CSRF$'.$prefix.'$', $token);

        return $token;
    }

    /**
     * Check if the CSRF token sent in the request is the same that the current in session
     *
     * @param string $prefix
     * @param string|null $tokenKey
     * @param string|null $tokenValue
     * @return boolean
     * @throws \Phalcon\Security\Exception
     */
    public function checkToken($prefix = '', $tokenKey = null, $tokenValue = null)
    {
        /* Type check */
        if(is_string($prefix) === false) {
            throw new \Phalcon\Security\Exception('Invalid prefix.');
        }

        if(is_null($tokenKey) === false && is_string($tokenKey) === false) {
            throw new \Phalcon\Security\Exception('Invalid parameter type.');
        }

        if(is_null($tokenValue) === false && is_string($tokenValue) === false) {
            throw new \Phalcon\Security\Exception('Invalid parameter type.');
        }

        /* Get session service */
        if(is_object($this->_dependencyInjector) === false) {
            throw new \Phalcon\Security\Exception('A dependency injection container is required to access the \'session\' service');
        }

        $session = $this->_dependencyInjector->getShared('session');
        if(is_object($session) === false ||
            $session instanceof \Phalcon\Session\AdapterInterface === false) {
            throw new \Phalcon\Security\Exception('Session service is unavailable.');
        }

        /* Get token data */
        if(is_null($tokenKey) === true) {
            $tokenKey = $session->get('$PHALCON\CSRF\KEY$'.$prefix.'$');
        }

        if(is_null($tokenValue) === true) {
            $request = $this->_dependencyInjector->getShared('service');

            if(is_object($request) === false ||
                $request instanceof \Phalcon\Http\RequestInterface === false) {
                throw new \Phalcon\Security\Exception('Request service is unavailable.');
            }

            //We always check if the value is correct in post
            $tokenValue = $request->getPost($tokenKey);
        }

        $session_token = $session->get('$PHALCON/CSRF$'.$prefix.'$');

        //The value is the same?
        return ($tokenValue === $session_token ? true : false);
    }

    /**
     * Returns the value of the CSRF token in session
     *
     * @param string $prefix
     * @return string
     * @throws \Phalcon\Security\Exception
     */
    public function getSessionToken($prefix = '')
    {
        if(is_string($prefix) === false) {
            throw new \Phalcon\Security\Exception('Invalid prefix.');
        }

        if(is_object($this->_dependencyInjector) === false) {
            throw new \Phalcon\Security\Exception('A dependency injection container is required to access the \'session\' service');
        }

        $session = $this->_dependencyInjector->getShared('session');
        if(is_object($session) === false ||
            $session instanceof \Phalcon\Session\AdapterInterface === false) {
            throw new \Phalcon\Security\Exception('Session service is unavailable.');
        }

        return $session->get('$PHALCON/CSRF$'.$prefix.'$');
    }
}
?>

Alternatively, you can generate the session token once, then just re-use it for every form. This is still sufficient protection against CSRF attacks.

quasipickle is completely right!

To have a secure protection against CSRF attacks we only need to generate ONE token and use it for ENTIRE session.

1. Generate token/key only once - when the user logs in.
2. Use this token for entire session in each form. It does not matter how many tabs/forms is being opened.

3. Delete token from session once - when user logs out from the app.

   Some post from stackoverflow I have read:

1. https://stackoverflow.com/questions/20057225/csrf-token-collisions-with-multiple-tabs
2. https://stackoverflow.com/questions/15829977/too-many-csrf-tokens-generated-php-how-do-i-deal-with-them?rq=1
3. https://stackoverflow.com/questions/10466241/new-csrf-token-per-request-or-not
4. https://stackoverflow.com/questions/20587746/one-token-vs-multiple-tokens-to-prevent-csrf-attacks
5. https://stackoverflow.com/questions/6724365/csrf-protection-question?rq=1
6. https://stackoverflow.com/questions/6929449/are-we-really-secured-from-csrf?rq=1

A quatation from official OWASP document:

In general, developers need only generate this token once for the current session. After initial generation of this token, the value is stored in the session and is utilized for each subsequent request until the session expires. When a request is issued by the end-user, the server-side component must verify the existence and validity of the token in the request as compared to the token found in the session. If the token was not found within the request or the value provided does not match the value within the session, then the request should be aborted, token should be reset and the event logged as a potential CSRF attack in progress.

In fact OWASP says ones can consider having different token for each request but I stay with one for entire session.

To further enhance the security of this proposed design, consider randomizing the CSRF token parameter name and or value for each request. Implementing this approach results in the generation of per-request tokens as opposed to per-session tokens. Note, however, that this may result in usability concerns. For example, the "Back" button browser capability is often hindered as the previous page may contain a token that is no longer valid. Interaction with this previous page will result in a CSRF false positive security event at the server. Regardless of the approach taken, developers are encouraged to protect the CSRF token the same way they protect authenticated session identifiers, such as the use of SSLv3/TLS.

Source: https://www.owasp.org/index.php/Cross-SiteRequestForgery%28CSRF%29PreventionCheatSheet#GeneralRecommendation:SynchronizerTokenPattern

All in all @Milad should decide whether to use per-session or per-request tokens.

Thanks All, I'd go for one-time generated token per session because it's easier, hassle-free and yet it's secure enough. But I don't know to whom I should award the correct answer.

Thank you all.

Perfect solution! Thank a lot =)

Hey Milad,

when do you set the session tokens? I only see you checking those but not saving them. I also made a quick test and it didn't work for me.

Thank you all for your input.

I'll use the one token per session.

I have solved it using this app/library/security.php:

  <?php

  class Security extends \Phalcon\Security
  {
      public function getTokenKey($numberBytes = 13)
      {
          $key = '$PHALCON/CSRF/KEY$';

          $tokenKey = \Phalcon\DI::getDefault()->get('session')->$key;

          if ($tokenKey)
          {
              return $tokenKey;
          }

          return parent::getTokenKey($numberBytes);
      }

      public function getToken($numberBytes = 32)
      {
          $key = '$PHALCON/CSRF$';

          $token = \Phalcon\DI::getDefault()->get('session')->$key;

          if ($token)
          {
              return $token;
          }

          return parent::getToken($numberBytes);
      }
  }

And in the bootstrapper index.php

  $di->set('security', function() {
      $security = new Security();
      return $security;
  }, true);

Best regards

On login. Just set token once when user signs in and use it.

Do you have a code snippet, because I tried that. Also doesn't this mean a user could for example delete posts to whom he doesn't have access. While when using one token per page this token + the view accesss protected it?

On login. Just set token once when user signs in and use it.

Found the issue.

\Phalcon\DI::getDefault()->get('session')->$key;

needs to be

\Phalcon\DI::getDefault()->get('session')->get($key);

Unfortuntately it didn't throw any error.

This code is for PhalconPHP 1.3, not 2.0 so be advised.

Also the generation happens in the very same file:

class Security extends \Phalcon\Security
{
  public function getTokenKey($numberBytes = 13)
  {
      $key = '$PHALCON/CSRF/KEY$';

      $tokenKey = \Phalcon\DI::getDefault()->get('session')->$key;

      if ($tokenKey)
      {
          return $tokenKey;
      }

      return parent::getTokenKey($numberBytes);
  }

  public function getToken($numberBytes = 32)
  {
      $key = '$PHALCON/CSRF$';

      $token = \Phalcon\DI::getDefault()->get('session')->$key;

      if ($token)
      {
          return $token;
      }

      return parent::getToken($numberBytes);
  }
}

The lines:

return parent::getTokenKey($numberBytes);

and

return parent::getToken($numberBytes);

Both generates and return token and token key if they are not generated begore, so they are generated when they are needed for the first time (not after login necessarily) and they are used until the session ends or expires.

Of course the parent functions come from \Phalcon\Security.

You initialize the Security class in index.php

$di->set('security', function() {
    $security = new Security();
    return $security;
}, true);

Regards

Thank you, but the issue wasn't with how I tried to use it. It also doesn't require you to set it on login. The issue was with the syntax where there was an error, see previous post.

This code is for PhalconPHP 1.3, not 2.0 so be advised.

Also the generation happens in the very same file:

class Security extends \Phalcon\Security { public function getTokenKey($numberBytes = 13) { $key = '$PHALCON/CSRF/KEY$';

    $tokenKey = \Phalcon\DI::getDefault()->get('session')->$key;

    if ($tokenKey)
    {
        return $tokenKey;
    }

    return parent::getTokenKey($numberBytes);
}

public function getToken($numberBytes = 32)
{
    $key = '$PHALCON/CSRF$';

    $token = \Phalcon\DI::getDefault()->get('session')->$key;

    if ($token)
    {
        return $token;
    }

    return parent::getToken($numberBytes);
}

}

The lines:

return parent::getTokenKey($numberBytes);

and

return parent::getToken($numberBytes);

Both generates and return token and token key if they are not generated begore, so they are generated when they are needed for the first time (not after login necessarily) and they are used until the session ends or expires.

Of course the parent functions come from \Phalcon\Security.

You initialize the Security class in index.php

$di->set('security', function() { $security = new Security(); return $security; }, true);

Regards

Thanks Milad!

I'm using Phalcon 2.0.6 and I've noticed that when the token is checked it's regenerated. So I've also overridden the checkToken method changing this default behavior.

class Security extends \Phalcon\Security
{
    public function getTokenKey($numberBytes = 13)
    {
        $key = '$PHALCON/CSRF/KEY$';

        $tokenKey = \Phalcon\DI::getDefault()->get('session')->get($key);

        if ($tokenKey) {
            return $tokenKey;
        }

        return parent::getTokenKey($numberBytes);
    }

    public function getToken($numberBytes = 32)
    {
        $key = '$PHALCON/CSRF$';

        $token = \Phalcon\DI::getDefault()->get('session')->get($key);

        if ($token) {
            return $token;
        }

        return parent::getToken($numberBytes);
    }

    public function checkToken($tokenKey = null, $tokenValue = null, $destroyIfValid = false)
    {
        $tokenKey = $tokenKey ?: $this->getTokenKey();
        $tokenValue = $tokenValue ?: $this->getToken();
        return parent::checkToken($tokenKey, $tokenValue, $destroyIfValid);
    }

}

Thank you all for your input.

I'll use the one token per session.

I have solved it using this app/library/security.php:

  <?php

  class Security extends \Phalcon\Security
  {
      public function getTokenKey($numberBytes = 13)
      {
          $key = '$PHALCON/CSRF/KEY$';

          $tokenKey = \Phalcon\DI::getDefault()->get('session')->$key;

          if ($tokenKey)
          {
              return $tokenKey;
          }

          return parent::getTokenKey($numberBytes);
      }

      public function getToken($numberBytes = 32)
      {
          $key = '$PHALCON/CSRF$';

          $token = \Phalcon\DI::getDefault()->get('session')->$key;

          if ($token)
          {
              return $token;
          }

          return parent::getToken($numberBytes);
      }
  }

And in the bootstrapper index.php

  $di->set('security', function() {
      $security = new Security();
      return $security;
  }, true);

Best regards

Great solution! I needed this for a multi level ajax form :)

This is how I use it in Phalcon 3:

use Phalcon\Di;

class Security extends \Phalcon\Security
{
    public function getTokenKey()
    {
        $tokenKey = Di::getDefault()->get('session')->get($this->_tokenKeySessionID);

        if ($tokenKey) {
            return $tokenKey;
        }

        return parent::getTokenKey();
    }

    public function getToken()
    {
        $token = Di::getDefault()->get('session')->get($this->_tokenValueSessionID);

        if ($token) {
            return $token;
        }

        return parent::getToken();
    }

    public function checkToken($tokenKey = null, $tokenValue = null, $destroyIfValid = false)
    {
        return parent::checkToken($tokenKey, $tokenValue, $destroyIfValid);
    }
}

The checkToken method should not fetch the key & value from the session, because the post value (which should be checked) will be overridden