Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ArrayIndexOutOfBoundsException in XorCsrfTokenRequestAttributeHandler #13310

Closed
mhankus opened this issue Jun 13, 2023 · 7 comments
Closed

ArrayIndexOutOfBoundsException in XorCsrfTokenRequestAttributeHandler #13310

mhankus opened this issue Jun 13, 2023 · 7 comments
Assignees
Labels
in: web An issue in web modules (web, webmvc) type: enhancement A general enhancement

Comments

@mhankus
Copy link

mhankus commented Jun 13, 2023

java.lang.ArrayIndexOutOfBoundsException is thrown in XorCsrfTokenRequestAttributeHandler during attack
Affects version spring-security 6.0.3 (I have not tested 6.1)

java.lang.ArrayIndexOutOfBoundsException: arraycopy: last destination index 36 out of bounds for byte[8]
        at org.springframework.security.web.csrf.XorCsrfTokenRequestAttributeHandler.xorCsrf(XorCsrfTokenRequestAttributeHandler.java:119) ~[spring-security-web-6.0.3.jar!/:6.0.3]
        at org.springframework.security.web.csrf.XorCsrfTokenRequestAttributeHandler.getTokenValue(XorCsrfTokenRequestAttributeHandler.java:99) ~[spring-security-web-6.0.3.jar!/:6.0.3]
        at org.springframework.security.web.csrf.XorCsrfTokenRequestAttributeHandler.resolveCsrfTokenValue(XorCsrfTokenRequestAttributeHandler.java:73) ~[spring-security-web-6.0.3.jar!/:6.0.3]
        at org.springframework.security.web.csrf.CsrfFilter.doFilterInternal(CsrfFilter.java:121) ~[spring-security-web-6.0.3.jar!/:6.0.3]
        at org.springframework.web.filter.OncePerRequestFilter.doFilter(OncePerRequestFilter.java:116) ~[spring-web-6.0.9.jar!/:6.0.9]
        at org.springframework.security.web.ObservationFilterChainDecorator$ObservationFilter.wrapFilter(ObservationFilterChainDecorator.java:185) ~[spring-security-web-6.0.3.jar!/:6.0.3]
        at org.springframework.security.web.ObservationFilterChainDecorator$ObservationFilter.doFilter(ObservationFilterChainDecorator.java:172) ~[spring-security-web-6.0.3.jar!/:6.0.3]
        at org.springframework.security.web.ObservationFilterChainDecorator$VirtualFilterChain.doFilter(ObservationFilterChainDecorator.java:133) ~[spring-security-web-6.0.3.jar!/:6.0.3]
        at org.springframework.security.web.header.HeaderWriterFilter.doHeadersAfter(HeaderWriterFilter.java:90) ~[spring-security-web-6.0.3.jar!/:6.0.3]
        at org.springframework.security.web.header.HeaderWriterFilter.doFilterInternal(HeaderWriterFilter.java:75) ~[spring-security-web-6.0.3.jar!/:6.0.3]
        at org.springframework.web.filter.OncePerRequestFilter.doFilter(OncePerRequestFilter.java:116) ~[spring-web-6.0.9.jar!/:6.0.9]
        at org.springframework.security.web.ObservationFilterChainDecorator$ObservationFilter.wrapFilter(ObservationFilterChainDecorator.java:185) ~[spring-security-web-6.0.3.jar!/:6.0.3]
        at org.springframework.security.web.ObservationFilterChainDecorator$ObservationFilter.doFilter(ObservationFilterChainDecorator.java:172) ~[spring-security-web-6.0.3.jar!/:6.0.3]
        at org.springframework.security.web.ObservationFilterChainDecorator$VirtualFilterChain.doFilter(ObservationFilterChainDecorator.java:133) ~[spring-security-web-6.0.3.jar!/:6.0.3]

To reproduce modify csrf token values on client side (cookie based tokens)

Expected behavior
getTokenValue should validate encoded token length and return null if value is incorrect. Generating stacktrace for exception is much more expensive and may impact performance.

@mhankus mhankus added status: waiting-for-triage An issue we've not yet triaged type: bug A general bug labels Jun 13, 2023
@RahulKumarNitP
Copy link
Contributor

Can I work on this issue?

@jzheaux
Copy link
Contributor

jzheaux commented Jul 3, 2023

@kevin2jordan, are you still interested in this issue? A PR would be most welcome!

@jzheaux jzheaux self-assigned this Jul 3, 2023
@jzheaux jzheaux added in: web An issue in web modules (web, webmvc) type: enhancement A general enhancement and removed status: waiting-for-triage An issue we've not yet triaged type: bug A general bug labels Jul 3, 2023
@RahulKumarNitP
Copy link
Contributor

@jzheaux Have submitted the
PR: #13550
Would appreciate the PR review and any comments/feedback for the PR.

jzheaux pushed a commit to RahulKumarNitP/spring-security that referenced this issue Aug 7, 2023
jzheaux added a commit to RahulKumarNitP/spring-security that referenced this issue Aug 7, 2023
- Add Reactive equivalent
- Update copyright

Issue spring-projectsgh-13310
@jzheaux jzheaux closed this as completed in e21da06 Aug 7, 2023
jzheaux added a commit that referenced this issue Aug 7, 2023
- Add Reactive equivalent
- Update copyright

Issue gh-13310
@maximilianschweitzer
Copy link

maximilianschweitzer commented Apr 10, 2024

This issue is not completely fixed yet. During an attack we still get

java.lang.ArrayIndexOutOfBoundsException: arraycopy: last destination index 36 out of bounds for byte[1]
	at org.springframework.security.web.csrf.XorCsrfTokenRequestAttributeHandler.xorCsrf(XorCsrfTokenRequestAttributeHandler.java:122)
	at org.springframework.security.web.csrf.XorCsrfTokenRequestAttributeHandler.getTokenValue(XorCsrfTokenRequestAttributeHandler.java:99)
	at org.springframework.security.web.csrf.XorCsrfTokenRequestAttributeHandler.resolveCsrfTokenValue(XorCsrfTokenRequestAttributeHandler.java:73)
	at org.springframework.security.web.csrf.CsrfFilter.doFilterInternal(CsrfFilter.java:121)
	at org.springframework.web.filter.OncePerRequestFilter.doFilter(OncePerRequestFilter.java:116)
	at org.springframework.security.web.FilterChainProxy$VirtualFilterChain.doFilter(FilterChainProxy.java:374)
	at org.springframework.web.filter.CharacterEncodingFilter.doFilterInternal(CharacterEncodingFilter.java:201)
	at org.springframework.web.filter.OncePerRequestFilter.doFilter(OncePerRequestFilter.java:116)
	at org.springframework.security.web.FilterChainProxy$VirtualFilterChain.doFilter(FilterChainProxy.java:374)
	at org.springframework.web.filter.CorsFilter.doFilterInternal(CorsFilter.java:91)

in version spring-security 6.2.1.

Specifically, the problem lies in the following line in the file XorCsrfTokenRequestAttributeHandler.java:

System.arraycopy(csrfBytes, 0, xoredCsrf, 0, csrfBytes.length);

Here, you're attempting to copy csrfBytes.length elements from csrfBytes into xoredCsrf. However, xoredCsrf is only as long as the smaller of the two arrays (randomBytes.length or csrfBytes.length), as determined by the preceding line:

byte[] xoredCsrf = new byte[len]; // len is the smaller of randomBytes.length or csrfBytes.length

This will cause an ArrayIndexOutOfBoundsException when csrfBytes.length is greater than len, because you're trying to copy more elements than xoredCsrf can hold.

To fix this, you should copy only len elements, not csrfBytes.length elements, since len is the length of the destination array xoredCsrf.

// Copy only len elements
System.arraycopy(csrfBytes, 0, xoredCsrf, 0, len);

@pyyx
Copy link

pyyx commented Apr 12, 2024

java.lang.ArrayIndexOutOfBoundsException: arraycopy: last destination index 36 out of bounds for byte[0]
	at java.lang.System.arraycopy(Native Method) ~[?:?]
	at org.springframework.security.web.csrf.XorCsrfTokenRequestAttributeHandler.xorCsrf(XorCsrfTokenRequestAttributeHandler.java:119) ~[spring-security-web-6.1.8.jar:6.1.8]

In version 6.1.8, I had the same problem as @maximilianschweitzer .Hopefully this will be resolved

@kratosmy
Copy link

Hi, @jzheaux, I have made a quick fix for this issue. Please kindly help review.

@sjohnr
Copy link
Member

sjohnr commented May 31, 2024

@maximilianschweitzer thanks for reporting the additional finding. I am considering this a bug and plan to backport the fix to OSS supported branches (5.8.x, 6.2.x and 6.3.x) as well. See gh-15184.

@kratosmy thanks for submitting a PR. I will provide feedback on the PR.

sjohnr added a commit that referenced this issue Jun 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: web An issue in web modules (web, webmvc) type: enhancement A general enhancement
Projects
None yet
Development

No branches or pull requests

7 participants