Affects: 6.1.13
if a client sends a malformed origin header in a CORS request to a spring boot application that looks like this:
curl 'http://localhost/sample \
-X 'OPTIONS' \
-H 'Origin: https://*@:;' \
The following exception will be thrown:
j.l.IllegalArgumentException: [https://*@:;] is not a valid HTTP URL
at o.s.w.u.UriComponentsBuilder.checkSchemeAndHost(UriComponentsBuilder.java:309)
at o.s.w.u.UriComponentsBuilder.fromOriginHeader(UriComponentsBuilder.java:371)
at o.s.w.cors.CorsUtils.isCorsRequest(CorsUtils.java:46)
at o.s.w.c.DefaultCorsProcessor.processRequest(DefaultCorsProcessor.java:86)
This exception is not handled, and bubbles out as a 500 Internal Server Error.
I would expect that the framework would handle the invalid input and reject the request with a 403 Forbidden with message "invalid cors request", like it does for many other kinds of invalid input.
The only workaround I have found is to register a custom corsFilter
bean, with a custom CorsProcessor
that handles the exception and rejects it.
Here's a unit test that fails due to this issue:
package testing;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
import org.springframework.http.HttpHeaders;
import org.springframework.http.HttpMethod;
import org.springframework.http.HttpStatus;
import org.springframework.mock.web.MockHttpServletRequest;
import org.springframework.mock.web.MockHttpServletResponse;
import org.springframework.web.cors.CorsConfiguration;
import org.springframework.web.cors.CorsProcessor;
import org.springframework.web.cors.DefaultCorsProcessor;
import java.io.IOException;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertFalse;
import static org.junit.jupiter.api.Assertions.assertTrue;
class SafeCorsProcessorTest {
private final CorsConfiguration corsConfiguration = new CorsConfiguration();
private final CorsProcessor processor = new DefaultCorsProcessor();
private final MockHttpServletResponse response = new MockHttpServletResponse();
@BeforeEach
void setUp() {
response.reset();
}
@Test
void processRequest() throws IOException {
// Given a valid OPTIONS request.
var request = new MockHttpServletRequest();
request.addHeader(HttpHeaders.ORIGIN, "http://localhost");
request.setMethod(HttpMethod.OPTIONS.name());
// When processRequest is called
var result = processor.processRequest(corsConfiguration, request, response);
// Then the result is true and the status code is 200 OK.
assertTrue(result);
assertEquals(HttpStatus.OK.value(), response.getStatus());
}
@Test
void processRequestInvalidOrigin() throws IOException {
// Given an OPTIONS request with invalid origin header.
var request = new MockHttpServletRequest();
request.addHeader(HttpHeaders.ORIGIN, "https://*@:;");
request.setMethod(HttpMethod.OPTIONS.name());
// WHen processRequest is called
var result = processor.processRequest(corsConfiguration, request, response);
// Then the result is false and status code is 403 Forbidden,
assertFalse(result);
assertEquals(HttpStatus.FORBIDDEN.value(), response.getStatus());
}
}
Comment From: sfc-gh-jzana
Note: I stumbled on https://github.com/spring-projects/spring-framework/issues/33639 in the 6.2.0 RC2 milestone which likely changes the exact call stack and exception details. but I still don't see any exception handling in CorsUtils, so I suspect the error will still occur.
Comment From: rstoyanchev
@sfc-gh-jzana, thanks for the additional detail on how you came across this. To be sure, you're not saying that there is a regression as a result of #33639, correct? In other words, you are just pointing out that there isn't any handling for this, and there has never been. I expect the origin header is typically well formed, and that's why this hasn't been noticed or reported as an issue.
Comment From: sfc-gh-jzana
@rstoyanchev - correct. This is a preexisting issue from before that change (I am using 6.1.13).
Agreed that this is not seen in normal use as the browser controls the ORIGIN header. We found this error through penetration testing of our API surface.
Comment From: IgorOffline
Hi, I created a pull request that addresses this issue.
Comment From: simonbasle
Closed in https://github.com/spring-projects/spring-framework/commit/8da31e1db76ee078735731765d8313e7d323692a
Comment From: IgorOffline
Thank you!
Comment From: sfc-gh-jzana
@simonbasle Thanks for the quick fix!