From 30d68f2de78ef70ebcb02c2d40c85587f384f618 Mon Sep 17 00:00:00 2001 From: Rossen Stoyanchev Date: Tue, 26 Nov 2019 16:21:48 +0000 Subject: [PATCH] Reject user names with "%2F" in STOMP Closes gh-23836 --- .../messaging/simp/SimpMessagingTemplate.java | 3 ++- .../simp/user/DefaultUserDestinationResolver.java | 3 ++- .../messaging/simp/SimpMessagingTemplateTests.java | 7 +++++++ .../simp/user/DefaultUserDestinationResolverTests.java | 10 ++++++++++ 4 files changed, 21 insertions(+), 2 deletions(-) diff --git a/spring-messaging/src/main/java/org/springframework/messaging/simp/SimpMessagingTemplate.java b/spring-messaging/src/main/java/org/springframework/messaging/simp/SimpMessagingTemplate.java index c3ddf8a894a1..bd157bd5f80a 100644 --- a/spring-messaging/src/main/java/org/springframework/messaging/simp/SimpMessagingTemplate.java +++ b/spring-messaging/src/main/java/org/springframework/messaging/simp/SimpMessagingTemplate.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2018 the original author or authors. + * Copyright 2002-2019 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -224,6 +224,7 @@ public void convertAndSendToUser(String user, String destination, Object payload throws MessagingException { Assert.notNull(user, "User must not be null"); + Assert.isTrue(!user.contains("%2F"), "Invalid sequence \"%2F\" in user name: " + user); user = StringUtils.replace(user, "/", "%2F"); destination = destination.startsWith("/") ? destination : "/" + destination; super.convertAndSend(this.destinationPrefix + user + destination, payload, headers, postProcessor); diff --git a/spring-messaging/src/main/java/org/springframework/messaging/simp/user/DefaultUserDestinationResolver.java b/spring-messaging/src/main/java/org/springframework/messaging/simp/user/DefaultUserDestinationResolver.java index 5f71ec962ba0..62729df07e93 100644 --- a/spring-messaging/src/main/java/org/springframework/messaging/simp/user/DefaultUserDestinationResolver.java +++ b/spring-messaging/src/main/java/org/springframework/messaging/simp/user/DefaultUserDestinationResolver.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2018 the original author or authors. + * Copyright 2002-2019 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -203,6 +203,7 @@ private ParseResult parseSubscriptionMessage(Message message, String sourceDe } Principal principal = SimpMessageHeaderAccessor.getUser(headers); String user = (principal != null ? principal.getName() : null); + Assert.isTrue(user == null || !user.contains("%2F"), "Invalid sequence \"%2F\" in user name: " + user); Set sessionIds = Collections.singleton(sessionId); return new ParseResult(sourceDestination, actualDestination, sourceDestination, sessionIds, user); } diff --git a/spring-messaging/src/test/java/org/springframework/messaging/simp/SimpMessagingTemplateTests.java b/spring-messaging/src/test/java/org/springframework/messaging/simp/SimpMessagingTemplateTests.java index 7b4343aac88c..2062f7ceafaa 100644 --- a/spring-messaging/src/test/java/org/springframework/messaging/simp/SimpMessagingTemplateTests.java +++ b/spring-messaging/src/test/java/org/springframework/messaging/simp/SimpMessagingTemplateTests.java @@ -36,6 +36,7 @@ import org.springframework.util.LinkedMultiValueMap; import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatIllegalArgumentException; /** * Unit tests for {@link org.springframework.messaging.simp.SimpMessagingTemplate}. @@ -86,6 +87,12 @@ public void convertAndSendToUserWithEncoding() { assertThat(headerAccessor.getDestination()).isEqualTo("/user/https:%2F%2Fjoe.openid.example.org%2F/queue/foo"); } + @Test // gh-23836 + public void convertAndSendToUserWithInvalidSequence() { + assertThatIllegalArgumentException().isThrownBy(() -> + this.messagingTemplate.convertAndSendToUser("joe%2F", "/queue/foo", "data")); + } + @Test public void convertAndSendWithCustomHeader() { Map headers = Collections.singletonMap("key", "value"); diff --git a/spring-messaging/src/test/java/org/springframework/messaging/simp/user/DefaultUserDestinationResolverTests.java b/spring-messaging/src/test/java/org/springframework/messaging/simp/user/DefaultUserDestinationResolverTests.java index c2887f5d1b25..888afe23960e 100644 --- a/spring-messaging/src/test/java/org/springframework/messaging/simp/user/DefaultUserDestinationResolverTests.java +++ b/spring-messaging/src/test/java/org/springframework/messaging/simp/user/DefaultUserDestinationResolverTests.java @@ -29,6 +29,7 @@ import org.springframework.util.StringUtils; import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatIllegalArgumentException; import static org.mockito.BDDMockito.given; import static org.mockito.Mockito.mock; @@ -113,6 +114,15 @@ public void handleSubscribeNoUser() { assertThat(actual.getUser()).isNull(); } + @Test // gh-23836 + public void handleSubscribeInvalidUserName() { + TestPrincipal user = new TestPrincipal("joe%2F"); + String sourceDestination = "/user/queue/foo"; + + Message message = createMessage(SimpMessageType.SUBSCRIBE, user, "123", sourceDestination); + assertThatIllegalArgumentException().isThrownBy(() -> this.resolver.resolveDestination(message)); + } + @Test public void handleUnsubscribe() { TestPrincipal user = new TestPrincipal("joe");