paragonie/anti-csrf

In login form if user has entered wrong username and password then from next time validateRequest() returning false

Closed this issue · 10 comments

I am facing an issue while executing login action from the second time.
I can see the issue is with validateRequest() function where once we find valid CSRF token then we unset the current CSRF token (unset($this->session[$this->sessionIndex][$index]);)
I am validating valid CSRF token in the beginning of the action. I know it will work well if I check for CSRF validity at the end of the function but this seems to be not expected.

Can you please help me with this issue? what is the use of unsetting CSRF token inside validateRequest() ?

Each token is intended to be used only once, and only on a given endpoint.

so this mean we need to call validateRequest() at the end of an endpoint when all the validations are complete?

Generally,

if (AntiCSRF::validateRequest()) { // Do this only once per HTTP request!
    // Now $_POST can be relied on
}

And don't try to reuse tokens for multiple requests. And don't try to validate more than once.

Hey Scott,

I'd like to ask you what kind of vulnerabilities it opens if the CSRF token is not removed after validation? I know that it is less secure but if this is not a critical security issue then personally, I'd prefer to allow the reuse of tokens for better user experience (when the same form is open on multiple tabs).

I am curious about your answer! And if you agree, I could create a PR to make the reuse of tokens to be configurable.

@paragonie-scott I learned from a post of Anthony Ferrara that session-based CSRF won't protect against replay attacks. In spite of this fact, I am curious if you are wiling to accept a PR that would make the reuse of tokens configurable?

I'm not sure you would even need a library in that case. If you need tokens to be reusable,

if (!isset($_SESSION['csrfToken'])) {
    $_SESSION['csrfToken'] = random_bytes(33);
}

if (isset($_POST['csrf'])) {
    if (!hash_equals($_SESSION['csrfToken'], $_POST['csrf'])) {
        throw new SecurityException('Invalid CSRF token');
    }
}

And then

echo '<input type="hidden" name="csrf" value="', base64_encode($_SESSION['csrfToken']), '" />', PHP_EOL;

Forgot:

class SecurityException extends Exception { }

I was doing something like that before but I was unsure if my token generation is secure (it wasn't of course). That's the reason I wanted to use your lib so that I don't have to worry about it anymore. :) So thank you for the example, I'll keep using what I was using before (with some modifications)!

@paragonie-scott I agree with you, but the enforced single-use really does wreak havoc with AJAX forms. To that end, I hacked up configurable single_use and added configurable age limits.

I think this is a good balance between security and convenience. You still get at minimum, one token per form, but they'll be valid long enough for a user to complete an AJAX form successfully with more than one POST. Plus it will expire aged tokens, which seems like a good idea regardless.

Sent a PR

This will be fixed in the next release, with the Reusable class.