From 5bc80fc094ed0cd308748e47b83a66cc64b5daff Mon Sep 17 00:00:00 2001 From: Sam Brannen Date: Sun, 4 Jun 2023 17:01:55 +0200 Subject: [PATCH] Disable SpEL selector support in WebSocket messaging by default This commit disables support for evaluating SpEL expressions from untrusted sources by default. Specifically, this applies to the SpEL-based 'selector' header support in WebSocket messaging, which includes the DefaultSubscriptionRegistry and the classes used to configure the 'selector' header name (SimpleBrokerMessageHandler and SimpleBrokerRegistration). The selector header support remains in place but will have to be explicitly enabled beginning with Spring Framework 6.1. For example, a custom implementation of WebSocketMessageBrokerConfigurer can override the configureMessageBroker() method and configure the selector header name as follows. registry.enableSimpleBroker().setSelectorHeaderName("selector"); Closes gh-30550 --- .../broker/DefaultSubscriptionRegistry.java | 18 +++++++++++------- .../broker/SimpleBrokerMessageHandler.java | 10 ++++++---- .../simp/config/SimpleBrokerRegistration.java | 10 ++++++---- .../DefaultSubscriptionRegistryTests.java | 10 +++++----- ...MessageBrokerConfigurationSupportTests.java | 12 ++++++------ .../StompWebSocketIntegrationTests.java | 2 +- 6 files changed, 35 insertions(+), 27 deletions(-) diff --git a/spring-messaging/src/main/java/org/springframework/messaging/simp/broker/DefaultSubscriptionRegistry.java b/spring-messaging/src/main/java/org/springframework/messaging/simp/broker/DefaultSubscriptionRegistry.java index 2cc06cb90ea..45af042a1e8 100644 --- a/spring-messaging/src/main/java/org/springframework/messaging/simp/broker/DefaultSubscriptionRegistry.java +++ b/spring-messaging/src/main/java/org/springframework/messaging/simp/broker/DefaultSubscriptionRegistry.java @@ -54,9 +54,11 @@ import org.springframework.util.StringUtils; * in memory and uses a {@link org.springframework.util.PathMatcher PathMatcher} * for matching destinations. * - *

As of 4.2, this class supports a {@link #setSelectorHeaderName selector} - * header on subscription messages with Spring EL expressions evaluated against - * the headers to filter out messages in addition to destination matching. + *

This class also supports an optional selector header on subscription + * messages with Spring Expression Language (SpEL) expressions evaluated against + * the headers to filter out messages in addition to destination matching. As of + * Spring Framework 6.1, the SpEL support is disabled by default, but it can be + * enabled by setting a {@linkplain #setSelectorHeaderName selector header name}. * * @author Rossen Stoyanchev * @author Sebastien Deleuze @@ -79,7 +81,7 @@ public class DefaultSubscriptionRegistry extends AbstractSubscriptionRegistry { private int cacheLimit = DEFAULT_CACHE_LIMIT; @Nullable - private String selectorHeaderName = "selector"; + private String selectorHeaderName; private volatile boolean selectorHeaderInUse; @@ -130,9 +132,11 @@ public class DefaultSubscriptionRegistry extends AbstractSubscriptionRegistry { * 

 	 * headers.foo == 'bar'
 	 *
- *

By default this is set to "selector". You can set it to a different - * name, or to {@code null} to turn off support for a selector header. - * @param selectorHeaderName the name to use for a selector header + *

By default the selector header name is set to {@code null} which disables + * this feature. You can set it to {@code "selector"} or a different name to + * enable support for a selector header. + * @param selectorHeaderName the name to use for a selector header, or {@code null} + * or blank to disable selector header support * @since 4.2 */ public void setSelectorHeaderName(@Nullable String selectorHeaderName) { diff --git a/spring-messaging/src/main/java/org/springframework/messaging/simp/broker/SimpleBrokerMessageHandler.java b/spring-messaging/src/main/java/org/springframework/messaging/simp/broker/SimpleBrokerMessageHandler.java index d91e5fce1cc..d6e4fcdb85e 100644 --- a/spring-messaging/src/main/java/org/springframework/messaging/simp/broker/SimpleBrokerMessageHandler.java +++ b/spring-messaging/src/main/java/org/springframework/messaging/simp/broker/SimpleBrokerMessageHandler.java @@ -61,7 +61,7 @@ public class SimpleBrokerMessageHandler extends AbstractBrokerMessageHandler { private Integer cacheLimit; @Nullable - private String selectorHeaderName = "selector"; + private String selectorHeaderName; @Nullable private TaskScheduler taskScheduler; @@ -172,11 +172,13 @@ public class SimpleBrokerMessageHandler extends AbstractBrokerMessageHandler { * 

 	 * headers.foo == 'bar'
 	 *
- *

By default this is set to "selector". You can set it to a different - * name, or to {@code null} to turn off support for a selector header. + *

By default the selector header name is set to {@code null} which disables + * this feature. You can set it to {@code "selector"} or a different name to + * enable support for a selector header. *

Setting this property has no effect if the underlying SubscriptionRegistry * is not an instance of {@link DefaultSubscriptionRegistry}. - * @param selectorHeaderName the name to use for a selector header + * @param selectorHeaderName the name to use for a selector header, or {@code null} + * or blank to disable selector header support * @since 4.3.17 * @see #setSubscriptionRegistry * @see DefaultSubscriptionRegistry#setSelectorHeaderName(String) diff --git a/spring-messaging/src/main/java/org/springframework/messaging/simp/config/SimpleBrokerRegistration.java b/spring-messaging/src/main/java/org/springframework/messaging/simp/config/SimpleBrokerRegistration.java index fb2ece63ce6..628914a9d78 100644 --- a/spring-messaging/src/main/java/org/springframework/messaging/simp/config/SimpleBrokerRegistration.java +++ b/spring-messaging/src/main/java/org/springframework/messaging/simp/config/SimpleBrokerRegistration.java @@ -38,7 +38,7 @@ public class SimpleBrokerRegistration extends AbstractBrokerRegistration { private long[] heartbeat; @Nullable - private String selectorHeaderName = "selector"; + private String selectorHeaderName; /** @@ -90,9 +90,11 @@ public class SimpleBrokerRegistration extends AbstractBrokerRegistration { * 

 	 * headers.foo == 'bar'
 	 *
- *

By default this is set to "selector". You can set it to a different - * name, or to {@code null} to turn off support for a selector header. - * @param selectorHeaderName the name to use for a selector header + *

By default the selector header name is set to {@code null} which disables + * this feature. You can set it to {@code "selector"} or a different name to + * enable support for a selector header. + * @param selectorHeaderName the name to use for a selector header, or {@code null} + * or blank to disable selector header support * @since 4.3.17 */ public void setSelectorHeaderName(@Nullable String selectorHeaderName) { diff --git a/spring-messaging/src/test/java/org/springframework/messaging/simp/broker/DefaultSubscriptionRegistryTests.java b/spring-messaging/src/test/java/org/springframework/messaging/simp/broker/DefaultSubscriptionRegistryTests.java index 5b111dd1e38..9416b2d94e5 100644 --- a/spring-messaging/src/test/java/org/springframework/messaging/simp/broker/DefaultSubscriptionRegistryTests.java +++ b/spring-messaging/src/test/java/org/springframework/messaging/simp/broker/DefaultSubscriptionRegistryTests.java @@ -253,7 +253,7 @@ class DefaultSubscriptionRegistryTests { } @Test - void registerSubscriptionWithSelectorHeaderEnabledByDefault() { + void registerSubscriptionWithSelectorHeaderEnabled() { String sessionId1 = "sess01"; String sessionId2 = "sess02"; String sessionId3 = "sess03"; @@ -264,6 +264,9 @@ class DefaultSubscriptionRegistryTests { String selector1 = "headers.foo == 'bar'"; String selector2 = "headers.foo == 'enigma'"; + // Explicitly enable selector support + this.registry.setSelectorHeaderName("selector"); + // Register subscription with matching selector header this.registry.registerSubscription(subscribeMessage(sessionId1, subscriptionId1, destination, selector1)); // Register subscription with non-matching selector header @@ -297,7 +300,7 @@ class DefaultSubscriptionRegistryTests { } @Test - void registerSubscriptionWithSelectorHeaderDisabled() { + void registerSubscriptionWithSelectorHeaderDisabledByDefault() { String sessionId1 = "sess01"; String sessionId2 = "sess02"; String sessionId3 = "sess03"; @@ -308,9 +311,6 @@ class DefaultSubscriptionRegistryTests { String selector1 = "headers.foo == 'bar'"; String selector2 = "headers.foo == 'enigma'"; - // Explicitly disable selector header support - this.registry.setSelectorHeaderName(null); - // Register subscription with matching selector header this.registry.registerSubscription(subscribeMessage(sessionId1, subscriptionId1, destination, selector1)); // Register subscription with non-matching selector header diff --git a/spring-websocket/src/test/java/org/springframework/web/socket/config/annotation/WebSocketMessageBrokerConfigurationSupportTests.java b/spring-websocket/src/test/java/org/springframework/web/socket/config/annotation/WebSocketMessageBrokerConfigurationSupportTests.java index 3f4ee1ef989..2a48863016c 100644 --- a/spring-websocket/src/test/java/org/springframework/web/socket/config/annotation/WebSocketMessageBrokerConfigurationSupportTests.java +++ b/spring-websocket/src/test/java/org/springframework/web/socket/config/annotation/WebSocketMessageBrokerConfigurationSupportTests.java @@ -165,8 +165,8 @@ class WebSocketMessageBrokerConfigurationSupportTests { } @Test - void selectorHeaderEnabledByDefault() { - ApplicationContext context = createContext(TestChannelConfig.class, TestConfigurer.class); + void selectorHeaderEnabled() { + ApplicationContext context = createContext(TestChannelConfig.class, SelectorHeaderConfigurer.class); SimpleBrokerMessageHandler simpleBrokerMessageHandler = simpleBrokerMessageHandler(context); assertThat(simpleBrokerMessageHandler.getSubscriptionRegistry()) @@ -176,8 +176,8 @@ class WebSocketMessageBrokerConfigurationSupportTests { } @Test - void selectorHeaderDisabled() { - ApplicationContext context = createContext(TestChannelConfig.class, SelectorHeaderConfigurer.class); + void selectorHeaderDisabledByDefault() { + ApplicationContext context = createContext(TestChannelConfig.class, TestConfigurer.class); SimpleBrokerMessageHandler simpleBrokerMessageHandler = simpleBrokerMessageHandler(context); assertThat(simpleBrokerMessageHandler.getSubscriptionRegistry()) @@ -282,8 +282,8 @@ class WebSocketMessageBrokerConfigurationSupportTests { @Override public void configureMessageBroker(MessageBrokerRegistry registry) { - // Explicitly disable selector header support - registry.enableSimpleBroker().setSelectorHeaderName(null); + // Explicitly enable selector header support + registry.enableSimpleBroker().setSelectorHeaderName("selector"); } } diff --git a/spring-websocket/src/test/java/org/springframework/web/socket/messaging/StompWebSocketIntegrationTests.java b/spring-websocket/src/test/java/org/springframework/web/socket/messaging/StompWebSocketIntegrationTests.java index ba91b7d7bad..a06c497e6ad 100644 --- a/spring-websocket/src/test/java/org/springframework/web/socket/messaging/StompWebSocketIntegrationTests.java +++ b/spring-websocket/src/test/java/org/springframework/web/socket/messaging/StompWebSocketIntegrationTests.java @@ -320,7 +320,7 @@ class StompWebSocketIntegrationTests extends AbstractWebSocketIntegrationTests { @Override public void configureMessageBroker(MessageBrokerRegistry configurer) { configurer.setApplicationDestinationPrefixes("/app"); - configurer.enableSimpleBroker("/topic", "/queue"); + configurer.enableSimpleBroker("/topic", "/queue").setSelectorHeaderName("selector"); } @Bean