spring-projects/spring-security

XorCsrfTokenRequestAttributeHandler always rejects valid CSRF token

BjoernKW opened this issue ยท 6 comments

Describe the bug
This code segment in XorCsrfTokenRequestAttributeHandler causes a valid CSRF token to always be rejected, which in turn results in an HTTP 401 status:

if (actualBytes.length < tokenSize) {
    return null;
}

// extract token and random bytes
int randomBytesSize = actualBytes.length - tokenSize;

If the actual token is Base64-encoded beforehand, it will match the tokenSize. However, even in that case the number of random bytes to XOR with will be 0, which again will lead to the token being rejected.

To Reproduce
Issue an HTTP POST request to a Spring Web MVC endpoint secured by Spring Security providing the CSRF token supplied by Spring Security.

Expected behavior
An actual token matching the expected token should be recognized as valid.

Sample
https://github.com/BjoernKW/sample-for-possible-csrf-token-bug-in-spring-security-6

I've now added a sample project for reproducing this issue under https://github.com/BjoernKW/sample-for-possible-csrf-token-bug-in-spring-security-6

Having the same issue.
Since Spring 6 CSRF is not working properly even after implementation changes.

In your example code the csrfTokenRequestHandler is wrong. You need to use delegate::handle instead of delegate.

// Use only the handle() method of XorCsrfTokenRequestAttributeHandler and the
// default implementation of resolveCsrfTokenValue() from CsrfTokenRequestHandler
CsrfTokenRequestHandler requestHandler = delegate::handle;

https://docs.spring.io/spring-security/reference/5.8/migration/servlet/exploits.html#_i_am_using_angularjs_or_another_javascript_framework

In your example code the csrfTokenRequestHandler is wrong. You need to use delegate::handle instead of delegate.

// Use only the handle() method of XorCsrfTokenRequestAttributeHandler and the
// default implementation of resolveCsrfTokenValue() from CsrfTokenRequestHandler
CsrfTokenRequestHandler requestHandler = delegate::handle;

https://docs.spring.io/spring-security/reference/5.8/migration/servlet/exploits.html#_i_am_using_angularjs_or_another_javascript_framework

I've just tried it that way. It doesn't work either.

Since the delegate::handle method reference can be replaced with a qualifier (delegate), csrfTokenRequestHandler(delegate) and csrfTokenRequestHandler(delegate::handle) are equivalent.

@BjoernKW Nope, they are not equivalent. Read the comment in the quote.

@BjoernKW Nope, they are not equivalent. Read the comment in the quote.

@lglauer ๐Ÿ‘ Thanks for pointing me in the right direction. Somehow, the change hasn't been picked up upon a first application restart. It's working now :-) I've just committed a working example under https://github.com/BjoernKW/sample-for-possible-csrf-token-bug-in-spring-security-6.

I still feel though that this should either still be the default behaviour or otherwise the documentation should be clearer and more explicit on how to achieve this. This is a very common use case and non-idempotent HTTP requests suddenly not working anymore with common JavaScript testing tools such as Postman can be quite confusing.