Using CSRF Protection With EE

I’d advise against using CI’s csrf_protection since, as you have noticed, EE has it’s own built-in CSRF protection. You should use EE’s function library’s form_declaration method instead of form_open, which will add the hidden CSRF token field (XID) automatically to your form.

$this->EE =& get_instance();

$this->EE->load->library('functions');

$form = $this->EE->functions->form_declaration(array('action' => '/something'));

//etc.

$form .= form_close();

echo $form;

If you take this approach, you’ll have to do the secure forms check on your own in your form action:

if ( ! $this->EE->security->secure_forms_check($this->EE->input->post('XID')))
{
    show_error('INVALID XID');
}

Thank you, however, I have not wrapped my head around this solution.

Since the custom controller(s) are handled by CI directly (as opposed to EE) EE will not parse the form. So the output of your suggested coding when viewing the source is as follows:

<form method="post" action="/something"  >
    <div class='hiddenFields'>
        <input type="hidden" name="XID" value="{XID_HASH}" />
        <input type="hidden" name="site_id" value="1" />
    </div>
</form>

The {XID_HASH} does not get parsed into a value.

I like this method, but in order to use it I suppose I need to know the how to access the EE equivalent of CI’s:

<input type="hidden" name="<?php echo $this->security->get_csrf_token_name(); ?>" value="<?php echo $this->security->get_csrf_hash(); ?>" >

It is kinda tricky switching between EE’s template_group/template handling of uri and CI’s controller/method, but once I was this issue gets squared away this (really any) web application will have sweet potential!

Ah, since you’re not going through the EE template parser, it’s not getting parsed. You can do this instead and forgo the form_declaration function:

<input type="hidden" name="XID" value="<?php echo $this->EE->functions->add_form_security_hash('{XID_HASH}'); ?>" >

Cool, getting closer, thank you. The trouble now is the I can not seem to get the secure_forms_check method to “pass” or “fail” to verify the code is working as expected. So to recap, I put the following into my CI view:

<input type="hidden" name="<?php echo $this->security->get_csrf_token_name(); ?>" value="<?php echo $this->security->get_csrf_hash(); ?>" >

Then in the controller/method the form (view) points to I created a conditional that only seems to return TRUE.

echo '<pre>';
print_r($_POST);
echo '</pre>

<p>’;</p>

<p>echo $this->input->post(‘XID’) . ’
‘;</p>

<p>// if ($this->security->secure_forms_check($this->input->post(‘XID’)))
if ($this->security->secure_forms_check(‘xid-that-should-fail’))
{
    echo ‘xid matches’ . ’
‘;
}
else
{
    show_error(‘INVALID XID’);
    echo ‘xid DOES NOT match’. ’
‘;
}</p>

<p>exit();

In the EE CP > Admin > Security and Privacy > Security and Sessions section, do you have “Process form data in Secure Mode?” set to yes?

Yes. (Process form data in Secure Mode is set to Yes)

Pop open system/expressionengine/core/EE_Security.php and do some var_dump’n in the check_xid method to see why it’s giving you false positives.

Ok, thanks for sticking with me so far. I opened EE_Security.php and dumped at different stages. The method called by:

$this->security->check_xid

yields the results expected (I can also see this in the security_hashes table), however, the controller methods do not seem to grab these results!?

if ($this->security->secure_forms_check($this->input->post('XID')))
{
    echo 'secure_forms_check returned true';
}
else
{
    echo 'secure_forms_check returned false';
}

Is it possible that somehow the controller is not referring to the current instance of the EE super global? I am confused still. In other words the var_dump indicates the security methods are performing as expected, however, their results are not passed as expected to the controller because I can change the conditional from:

if ($this->security->secure_forms_check($this->input->post('XID')))

to:

if ($this->security->secure_forms_check('should-return-false'))

and in either situation the controller conditional evaluates to true (even though the EE_Security.php file methods evaluate this to true and false respectively).

Note: The controller __construct() is the only reference to:

$this->EE =& get_instance();

That is weird! I’m not sure I have anything else to offer :/

No worries, thanks for your help thus far. I will get through it, perhaps by stepping back and looking at a different angle.

Brian,

Since what you are trying to do here is well beyond the scope of technical support and this conversation is well underway, I’m going to migrate this into Development and Programming to keep the discussion going.

Cheers,

The false positive seems to stem from EE_Security.php line 81 in the check_xid method that involves a conditional (to return FALSE) that does not seem to trigger when the hash is NOT found in the DB.

if ($total === 0)
{
    return FALSE;
}

If the code is modified to have two “==” (loose comparison operator) as opposed to three “===” (strict comparison operator), then the method returns the results I expect, here is why:

var_dump($total) indicates that $total is a string (the count of the specified hash in the security_hashes table).

Loose comparison of string “0” to integer 0 results in boolean TRUE, whereas, the strict comparison results in boolean FALSE. See PHP manual for comparison table.

The strict comparison of $total to 0 will never occur!

Making this change will resolve the question I posed, however, I don’t like modifying core files, but I am not sure how to proceed otherwise …

PHP is loosely typed (vs strongly typed) meaning you don’t have to declare the type and name of the variable before using it. An alternative could be something like adding (int) to line 75 like so:

$total = (int) $EE->db->where('hash', $xid)

Then the strict comparison would be comparing integer-to-integer and the conditional would evaluate as expected. Any other suggestions, perhaps someone with more expertise than I can confirm my conclusions and if necessary, notify EllisLabs as this effects security?

Good catch. You should totally file this in the EE Bug Tracker.

I reported the bug, you can follow it here.