From 867407046494432c760e9671dbd87b2d577b8441 Mon Sep 17 00:00:00 2001 From: rstoyanchev Date: Fri, 29 Apr 2022 10:41:16 +0100 Subject: [PATCH] CVE-2022-22971 - Ignore invalid connect frame Closes gh-28443 (cherry picked from commit dc2947c52df18d5e99cad03383f7d6ba13d031fd) --- build.gradle | 1 + .../broker/SimpleBrokerMessageHandler.java | 10 ++++++- .../stomp/StompBrokerRelayMessageHandler.java | 8 +++++- .../StompBrokerRelayMessageHandlerTests.java | 28 ++++++++++++++++++- 4 files changed, 44 insertions(+), 3 deletions(-) diff --git a/build.gradle b/build.gradle index a54185c90057..038ec357932e 100644 --- a/build.gradle +++ b/build.gradle @@ -145,6 +145,7 @@ configure(allprojects) { project -> exclude group:'org.hamcrest', module:'hamcrest-core' } testCompile("org.hamcrest:hamcrest-all:${hamcrestVersion}") + testCompile "org.assertj:assertj-core:3.18.1" sniffer("org.codehaus.mojo:animal-sniffer-ant-tasks:${snifferVersion}") javaApiSignature("org.codehaus.mojo.signature:java16:1.1@signature") // API level from JDK 6 update 18 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 21bb88dfab2e..17e927e45b16 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 @@ -1,5 +1,5 @@ /* - * Copyright 2002-2018 the original author or authors. + * Copyright 2002-2022 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. @@ -296,6 +296,14 @@ protected void handleMessageInternal(Message message) { } else if (SimpMessageType.CONNECT.equals(messageType)) { logMessage(message); + if (sessionId != null) { + if (this.sessions.get(sessionId) != null) { + if (logger.isWarnEnabled()) { + logger.warn("Ignoring CONNECT in session " + sessionId + ". Already connected."); + } + return; + } + } long[] clientHeartbeat = SimpMessageHeaderAccessor.getHeartbeat(headers); long[] serverHeartbeat = getHeartbeatValue(); Principal user = SimpMessageHeaderAccessor.getUser(headers); diff --git a/spring-messaging/src/main/java/org/springframework/messaging/simp/stomp/StompBrokerRelayMessageHandler.java b/spring-messaging/src/main/java/org/springframework/messaging/simp/stomp/StompBrokerRelayMessageHandler.java index dcf4a7a2825b..14fab85e115b 100644 --- a/spring-messaging/src/main/java/org/springframework/messaging/simp/stomp/StompBrokerRelayMessageHandler.java +++ b/spring-messaging/src/main/java/org/springframework/messaging/simp/stomp/StompBrokerRelayMessageHandler.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2017 the original author or authors. + * Copyright 2002-2022 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. @@ -494,6 +494,12 @@ else if (accessor instanceof SimpMessageHeaderAccessor) { } if (StompCommand.CONNECT.equals(command)) { + if (this.connectionHandlers.get(sessionId) != null) { + if (logger.isWarnEnabled()) { + logger.warn("Ignoring CONNECT in session " + sessionId + ". Already connected."); + } + return; + } if (logger.isDebugEnabled()) { logger.debug(stompAccessor.getShortLogMessage(EMPTY_PAYLOAD)); } diff --git a/spring-messaging/src/test/java/org/springframework/messaging/simp/stomp/StompBrokerRelayMessageHandlerTests.java b/spring-messaging/src/test/java/org/springframework/messaging/simp/stomp/StompBrokerRelayMessageHandlerTests.java index 5109f9d47841..d1b600ad9ec9 100644 --- a/spring-messaging/src/test/java/org/springframework/messaging/simp/stomp/StompBrokerRelayMessageHandlerTests.java +++ b/spring-messaging/src/test/java/org/springframework/messaging/simp/stomp/StompBrokerRelayMessageHandlerTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2015 the original author or authors. + * Copyright 2002-2022 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. @@ -44,6 +44,8 @@ import org.springframework.util.concurrent.ListenableFuture; import org.springframework.util.concurrent.ListenableFutureTask; +import static org.assertj.core.api.Assertions.assertThat; + /** * Unit tests for StompBrokerRelayMessageHandler. * @@ -220,6 +222,30 @@ public void systemSubscription() throws Exception { assertSame(message, captor.getValue()); } + @Test + public void alreadyConnected() { + + this.brokerRelay.start(); + + Message connect = connectMessage("sess1", "joe"); + this.brokerRelay.handleMessage(connect); + + assertThat(this.tcpClient.getSentMessages().size()).isEqualTo(2); + + StompHeaderAccessor headers1 = this.tcpClient.getSentHeaders(0); + assertThat(headers1.getCommand()).isEqualTo(StompCommand.CONNECT); + assertThat(headers1.getSessionId()).isEqualTo(StompBrokerRelayMessageHandler.SYSTEM_SESSION_ID); + + StompHeaderAccessor headers2 = this.tcpClient.getSentHeaders(1); + assertThat(headers2.getCommand()).isEqualTo(StompCommand.CONNECT); + assertThat(headers2.getSessionId()).isEqualTo("sess1"); + + this.brokerRelay.handleMessage(connect); + + assertThat(this.tcpClient.getSentMessages().size()).isEqualTo(2); + assertThat(this.outboundChannel.getMessages()).isEmpty(); + } + private Message connectMessage(String sessionId, String user) { StompHeaderAccessor headers = StompHeaderAccessor.create(StompCommand.CONNECT); headers.setSessionId(sessionId);