CorsUtils.isCorsRequest throws unhandled IllegalArgumentException and returns 500 Internal Server Error on malfomed Origin header

[sfc-gh-jzana]

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());
    }
}

[sfc-gh-jzana]

Note: I stumbled on #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.

[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.

[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.

[IgorOffline]

Hi, I created a pull request that addresses this issue.

[simonbasle]

Closed in 8da31e1

[IgorOffline]

Thank you!

[sfc-gh-jzana]

@simonbasle Thanks for the quick fix!