Skip to content

Commit

Permalink
Fix possible ArrayIndexOutOfBoundsException in XorCsrfTokenRequestAtt…
Browse files Browse the repository at this point in the history
…ributeHandler and XorCsrfTokenUtils
  • Loading branch information
kratosmy committed Apr 27, 2024
1 parent 1825dcb commit fe3c5bf
Show file tree
Hide file tree
Showing 4 changed files with 27 additions and 4 deletions.
Expand Up @@ -56,13 +56,16 @@ static String getTokenValue(String actualToken, String token) {
System.arraycopy(actualBytes, randomBytesSize, xoredCsrf, 0, tokenSize);

byte[] csrfBytes = xorCsrf(randomBytes, xoredCsrf);
return Utf8.decode(csrfBytes);
return (csrfBytes != null) ? Utf8.decode(csrfBytes) : null;
}

private static byte[] xorCsrf(byte[] randomBytes, byte[] csrfBytes) {
static byte[] xorCsrf(byte[] randomBytes, byte[] csrfBytes) {
if (csrfBytes.length < randomBytes.length) {
return null;
}
int len = Math.min(randomBytes.length, csrfBytes.length);
byte[] xoredCsrf = new byte[len];
System.arraycopy(csrfBytes, 0, xoredCsrf, 0, csrfBytes.length);
System.arraycopy(csrfBytes, 0, xoredCsrf, 0, len);
for (int i = 0; i < len; i++) {
xoredCsrf[i] ^= randomBytes[i];
}
Expand Down
Expand Up @@ -32,6 +32,7 @@
import org.springframework.security.web.csrf.MissingCsrfTokenException;

import static org.assertj.core.api.Assertions.assertThatExceptionOfType;
import static org.assertj.core.api.Assertions.assertThatNoException;
import static org.mockito.Mockito.mock;

/**
Expand Down Expand Up @@ -141,6 +142,13 @@ public void preSendWhenUnsubscribeThenIgnores() {
this.interceptor.preSend(message(), this.channel);
}

@Test
public void preSendWhenCsrfBytesIsLongerThanRandomBytesThenArrayIndexOutOfBoundsExceptionWillNotBeThrown() {
this.messageHeaders.setNativeHeader(this.token.getHeaderName(), XOR_CSRF_TOKEN_VALUE);
this.messageHeaders.getSessionAttributes().put(CsrfToken.class.getName(), this.token);
assertThatNoException().isThrownBy(() -> this.interceptor.preSend(message(), this.channel));
}

private Message<String> message() {
return MessageBuilder.withPayload("message").copyHeaders(this.messageHeaders.toMap()).build();
}
Expand Down
Expand Up @@ -119,7 +119,7 @@ private static byte[] xorCsrf(byte[] randomBytes, byte[] csrfBytes) {
}
int len = Math.min(randomBytes.length, csrfBytes.length);
byte[] xoredCsrf = new byte[len];
System.arraycopy(csrfBytes, 0, xoredCsrf, 0, csrfBytes.length);
System.arraycopy(csrfBytes, 0, xoredCsrf, 0, len);
for (int i = 0; i < len; i++) {
xoredCsrf[i] ^= randomBytes[i];
}
Expand Down
Expand Up @@ -30,6 +30,7 @@
import static org.assertj.core.api.Assertions.assertThat;
import static org.assertj.core.api.Assertions.assertThatIllegalArgumentException;
import static org.assertj.core.api.Assertions.assertThatIllegalStateException;
import static org.assertj.core.api.Assertions.assertThatNoException;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.BDDMockito.willAnswer;
import static org.mockito.Mockito.mock;
Expand Down Expand Up @@ -216,6 +217,17 @@ public void resolveCsrfTokenIsInvalidThenReturnsNull() {
assertThat(tokenValue).isNull();
}

@Test
public void resolveCsrfTokenValueWhenCsrfBytesIsLongerThanRandomBytesThenArrayIndexOutOfBoundsExceptionWillNotBeThrown() {
this.request.setParameter(this.token.getParameterName(), XOR_CSRF_TOKEN_VALUE);
CsrfToken csrfToken = new DefaultCsrfToken("headerName", "paramName", "ABCDE");
// @formatter:off
assertThatNoException().isThrownBy(() -> {
this.handler.resolveCsrfTokenValue(this.request, csrfToken);
});
// @formatter:on
}

private static Answer<Void> fillByteArray() {
return (invocation) -> {
byte[] bytes = invocation.getArgument(0);
Expand Down

0 comments on commit fe3c5bf

Please sign in to comment.