We have moved our forum to GitHub Discussions. For questions about Phalcon v3/v4/v5 you can visit here and for Phalcon v6 here.

Phalcon CSRF in self submit form

Hi all, I have a problem in CSRF security functionality in form. So there is my setup:

Form class:

class SignupForm extends Base
{
    public function initialize()
    {
        $email = new Text('email');
            $email->setLabel($this->translation->_('E-Mail'))
                ->setAttribute('placeholder', $this->translation->_('E-Mail'))
                ->addValidators(array(
                    new PresenceOf(array(
                        'message' => $this->translation->_('The e-mail is required'),
                        'cancelOnFail' => true
                    )),
                    new Email(array(
                        'message' => $this->translation->_('The e-mail is not valid'),
                    ))
                ));
            $this->add($email);

            // Sign Up
            $this->add(new Submit('signUp', array(
                'value' => $this->translation->_('Sign up')
            )));

            $csrf = new Hidden('csrf');
            $csrf->addValidator(new Identical(array(
                'value' => $this->security->getSessionToken(),
                'message' => $this->translation->_('This request was aborted because it appears to be forged')
            )));
            $this->add($csrf);
    }
}

and form scritp:

{{ form('class': 'ui large form') }}
{{ form.render('csrf', ['value': security.getToken()]) }}
{{ form.render('email') }}
{{ form.render('signUp') }}
</form>

I show validation message on top of the form and let user correct it and submit again! But when I submit form on itself (get and post in one action), csrf value just populate from last post values while security.getToken() generate and save new fresh token in session bag (line {{ form.render("csrf", ["value": security.getToken()]) }} in script runs already)! so secound submit does not validate this old csrf with new generated token!

How can I force my hidden csrf field to get new value instead of populate old post value?

Thanks

edited May '15

Hi,

Could you try change your csrf element to this:

    {{ element.render('csrf', ['name': this.security.getTokenKey(), 'value': this.security.getToken()]) }}

And change your form element to this:

        $csrf->addValidator(
            new Identical([
            $this->security->checkToken() => 1,
            'message' => $this->translation->_('This request was aborted because it appears to be forged')
        ]));

Ah, I have not seen your edited comment.

I will try that.

Thanks.

Hi,

Could you try change your csrf element to this:

  {{ element.render('csrf', ['name': this.security.getTokenKey(), 'value': this.security.getToken()]) }}

And change your form element to this:

      $csrf->addValidator(
          new Identical([
          $this->security->checkToken() => 1,
          'message' => $this->translation->_('This request was aborted because it appears to be forged')
      ]));
edited May '15

Thank you Warwiks, tricky and sweet. It works.

Maybe I sould read source more than documentaions!

Regards

No problem at all, glad you found your solution. If you have any questions then please ask.

Sorry, I accidentally accept my comment so I delete it.

@Warosaurus last comment is the answer!

edited May '15

@Warosaurus Can you explain little more, how this setup works? Seams that it just bypas validator when I change session value manually and always returns true ! . (I record sessions in DB)

Thanks again.

No problem at all, glad you found your solution. If you have any questions then please ask.

I just tested and you're right. It does validate incorrect values.

I switched back to an older solution. Remove the validator and instead check the token in your controller. This is my controller now:


        $form = new AccountLoginForm();

        $this->view->form = $form;

        if ($this->request->isPost()) {

            if ($this->security->checkToken()) { // <- CSRF check

I just tested this changing the CSRF token in my session variable and it works. Let me know if it works for you.

Thanks for the heads up.

@Warosaurus you know what? $this->security->checkToken() always works but implementing OOP CSRF validator in form does not!

It's a big issue when you have many forms and I think that it's not controller responsibility to check form validation if we have \Phalcon\Form\Form abstract layer!

It back to bad code structure in validator so all problems related to these lines (https://github.com/phalcon/cphalcon/blob/master/phalcon/validation.zep#L112) When we define a field the first param picks as field name (not input name) and all validations use that name to retrives values then we cannot use dynamic field name to retrive value in CSRF check at all.

I implement this CSRF custom validator and it works for me:

Validator\Csrf.php

namespace Application\Validator;

use Phalcon\Validation;
use Phalcon\Validation\Message;
use Phalcon\Validation\Validator;
use Phalcon\Validation\ValidatorInterface;

class Csrf extends Validator implements ValidatorInterface
{
    const DEFAULT_MESSAGE = "CSRF Attack: This request was aborted!";

    /**
     * Executes the validation
     *
     * @param Validation $validator
     * @param string $field
     * @return boolean
     */
    public function validate(Validation $validator, $field)
    {
        $this->setOption('cancelOnFail', true);

        if(!$validator->security->checkToken())
        {
            $message = $this->getOption("message");
            if (empty($message)) {
                $message = $validator->translation->_(self::DEFAULT_MESSAGE);
            }

            $validator->appendMessage(new Message($message, $field, "Csrf"));
            return false;
        }

        return true;
    }
} 

Usage in Form class:

$csrf = new Hidden('csrf');
$csrf->addValidator(new Csrf(array(
'message' => $this->translation->_('This request was aborted because it appears to be forged')
)));
$this->add($csrf);

Volt Script:

{{ form.render('csrf', ['name': security.getTokenKey(), 'value': security.getToken()]) }}

Hope this helps.

Regards

Having the same issue, so thanks a lot for your workaround, but in my opinion, going OOP style is the more clean way and it should work too! Because actually it doesn't work I suppose this is a bug in Phalcon and it must be corrected.

Yes, I think so. Somthing smells wrong here :)

Having the same issue, so thanks a lot for your workaround, but in my opinion, going OOP style is the more clean way and it should work too! Because actually it doesn't work I suppose this is a bug in Phalcon and it must be corrected.

Have you filled a bug report yet?

Yes, I think so. Somthing smells wrong here :)

Having the same issue, so thanks a lot for your workaround, but in my opinion, going OOP style is the more clean way and it should work too! Because actually it doesn't work I suppose this is a bug in Phalcon and it must be corrected.

I do not think this is a bug... It seams it is a bad designed feature!

Have you filled a bug report yet?

Yes, I think so. Somthing smells wrong here :)

Having the same issue, so thanks a lot for your workaround, but in my opinion, going OOP style is the more clean way and it should work too! Because actually it doesn't work I suppose this is a bug in Phalcon and it must be corrected.



5.0k
edited Oct '15

Working... In Form:

        $csrf = new Hidden('csrf');
        $csrf->addValidator(new Identical(array(
            'value' => $this->security->getSessionToken(),
            'message' => 'CSRF validation failed'
        )));
        $csrf->clear();
        $this->add($csrf);

in View:

{{ form.render('csrf', ['value': security.getToken()]) }}