diff --git a/src/main/java/com/clearfolio/viewer/config/ViewerSecurityHeadersWebFilter.java b/src/main/java/com/clearfolio/viewer/config/ViewerSecurityHeadersWebFilter.java index 71cd7392..7902bfe5 100644 --- a/src/main/java/com/clearfolio/viewer/config/ViewerSecurityHeadersWebFilter.java +++ b/src/main/java/com/clearfolio/viewer/config/ViewerSecurityHeadersWebFilter.java @@ -108,6 +108,10 @@ private String normalizeFrameAncestors(String configured) { if (configured == null) { return "'self'"; } + if (configured.indexOf(';') >= 0 || configured.chars().anyMatch(Character::isISOControl)) { + throw new IllegalArgumentException( + "viewer.security.frame-ancestors contains an unsafe CSP character"); + } String trimmed = configured.trim(); if (trimmed.isEmpty()) { return "'self'"; @@ -117,4 +121,4 @@ private String normalizeFrameAncestors(String configured) { } return trimmed; } -} +} \ No newline at end of file diff --git a/src/test/java/com/clearfolio/viewer/config/ViewerSecurityHeadersWebFilterTest.java b/src/test/java/com/clearfolio/viewer/config/ViewerSecurityHeadersWebFilterTest.java index 0e62117e..4090574d 100644 --- a/src/test/java/com/clearfolio/viewer/config/ViewerSecurityHeadersWebFilterTest.java +++ b/src/test/java/com/clearfolio/viewer/config/ViewerSecurityHeadersWebFilterTest.java @@ -1,28 +1,30 @@ package com.clearfolio.viewer.config; -import static org.junit.jupiter.api.Assertions.assertNull; -import static org.junit.jupiter.api.Assertions.assertNotNull; import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertNull; +import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertTrue; +import java.util.concurrent.atomic.AtomicBoolean; + import org.junit.jupiter.api.Test; import org.springframework.http.HttpHeaders; import org.springframework.http.HttpMethod; import org.springframework.http.HttpStatus; import org.springframework.http.MediaType; +import org.springframework.http.server.RequestPath; +import org.springframework.http.server.reactive.ServerHttpRequest; import org.springframework.mock.http.server.reactive.MockServerHttpRequest; import org.springframework.mock.http.server.reactive.MockServerHttpResponse; import org.springframework.mock.web.server.MockServerWebExchange; -import org.springframework.web.server.WebFilterChain; import org.springframework.web.server.ServerWebExchange; -import org.springframework.http.server.reactive.ServerHttpRequest; -import org.springframework.http.server.RequestPath; +import org.springframework.web.server.WebFilterChain; import reactor.core.publisher.Mono; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; -import java.util.concurrent.atomic.AtomicBoolean; class ViewerSecurityHeadersWebFilterTest { @@ -133,6 +135,23 @@ void usesCustomFrameAncestorsValueWhenConfigured() { assertTrue(csp.contains("frame-ancestors https://example.test")); } + @Test + void rejectsFrameAncestorDirectiveAndHeaderInjection() { + IllegalArgumentException directiveInjection = assertThrows( + IllegalArgumentException.class, + () -> new ViewerSecurityHeadersWebFilter("https://trusted.example; script-src *") + ); + assertEquals("viewer.security.frame-ancestors contains an unsafe CSP character", + directiveInjection.getMessage()); + + IllegalArgumentException headerInjection = assertThrows( + IllegalArgumentException.class, + () -> new ViewerSecurityHeadersWebFilter("https://trusted.example\nX-Injected: true") + ); + assertEquals("viewer.security.frame-ancestors contains an unsafe CSP character", + headerInjection.getMessage()); + } + @Test void supportsHeadRequestsForViewerSurface() { ViewerSecurityHeadersWebFilter filter = new ViewerSecurityHeadersWebFilter("self"); @@ -198,4 +217,4 @@ void doesNotAddHeadersForNonViewerPaths() { assertNull(headers.getFirst("X-Content-Type-Options")); assertNull(headers.getFirst("Referrer-Policy")); } -} +} \ No newline at end of file