diff --git a/client/src/main/java/org/red5/client/net/rtmp/BaseRTMPClientHandler.java b/client/src/main/java/org/red5/client/net/rtmp/BaseRTMPClientHandler.java index 31b83a3c1..9fc412b2f 100644 --- a/client/src/main/java/org/red5/client/net/rtmp/BaseRTMPClientHandler.java +++ b/client/src/main/java/org/red5/client/net/rtmp/BaseRTMPClientHandler.java @@ -41,6 +41,7 @@ import org.red5.server.net.rtmp.event.SWFResponse; import org.red5.server.net.rtmp.event.ServerBW; import org.red5.server.net.rtmp.message.Header; +import org.red5.server.net.rtmp.message.Constants; import org.red5.server.net.rtmp.status.StatusCodes; import org.red5.server.service.Call; import org.red5.server.service.MethodNotFoundException; @@ -309,10 +310,20 @@ public IClientSharedObject getSharedObject(String name, boolean persistent) { @Override protected void onChunkSize(RTMPConnection conn, Channel channel, Header source, ChunkSize chunkSize) { log.debug("onChunkSize"); + int requestedChunkSize = chunkSize.getSize(); + + // Validate chunk size to prevent security vulnerabilities + if (requestedChunkSize < Constants.MIN_CHUNK_SIZE || requestedChunkSize > Constants.MAX_CHUNK_SIZE) { + log.warn("Invalid chunk size received: {}. Must be between {} and {}. Ignoring.", + requestedChunkSize, Constants.MIN_CHUNK_SIZE, Constants.MAX_CHUNK_SIZE); + // Do not set the invalid chunk size - keep the existing one + return; + } + // set read and write chunk sizes RTMP state = conn.getState(); - state.setReadChunkSize(chunkSize.getSize()); - state.setWriteChunkSize(chunkSize.getSize()); + state.setReadChunkSize(requestedChunkSize); + state.setWriteChunkSize(requestedChunkSize); log.info("ChunkSize configured: {}", chunkSize); } diff --git a/common/src/main/java/org/red5/server/api/Red5.java b/common/src/main/java/org/red5/server/api/Red5.java index bebf9702b..ef6ebbd73 100644 --- a/common/src/main/java/org/red5/server/api/Red5.java +++ b/common/src/main/java/org/red5/server/api/Red5.java @@ -14,6 +14,7 @@ import javax.management.openmbean.CompositeData; import org.red5.server.api.scope.IScope; +import org.red5.server.net.rtmp.message.Constants; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -282,7 +283,14 @@ public static Red5 from(CompositeData cd) { * @param targetChunkSize the chunk size to use */ public static void setTargetChunkSize(int targetChunkSize) { - Red5.targetChunkSize = targetChunkSize; + // Validate chunk size to prevent security vulnerabilities + if (targetChunkSize < Constants.MIN_CHUNK_SIZE || targetChunkSize > Constants.MAX_CHUNK_SIZE) { + log.warn("Invalid target chunk size: {}. Must be between {} and {}. Using default value of 128.", + targetChunkSize, Constants.MIN_CHUNK_SIZE, Constants.MAX_CHUNK_SIZE); + Red5.targetChunkSize = 128; // Use safe default + } else { + Red5.targetChunkSize = targetChunkSize; + } } /** diff --git a/common/src/main/java/org/red5/server/net/rtmp/RTMPHandler.java b/common/src/main/java/org/red5/server/net/rtmp/RTMPHandler.java index c61b63f50..4014d4223 100644 --- a/common/src/main/java/org/red5/server/net/rtmp/RTMPHandler.java +++ b/common/src/main/java/org/red5/server/net/rtmp/RTMPHandler.java @@ -44,6 +44,7 @@ import org.red5.server.net.rtmp.event.SetBuffer; import org.red5.server.net.rtmp.event.StreamActionEvent; import org.red5.server.net.rtmp.message.Header; +import org.red5.server.net.rtmp.message.Constants; import org.red5.server.net.rtmp.status.Status; import org.red5.server.net.rtmp.status.StatusObject; import org.red5.server.net.rtmp.status.StatusObjectService; @@ -146,6 +147,15 @@ public void setDispatchStreamActions(boolean dispatchStreamActions) { protected void onChunkSize(RTMPConnection conn, Channel channel, Header source, ChunkSize chunkSize) { int requestedChunkSize = chunkSize.getSize(); log.debug("Chunk size: {}", requestedChunkSize); + + // Validate chunk size to prevent security vulnerabilities + if (requestedChunkSize < Constants.MIN_CHUNK_SIZE || requestedChunkSize > Constants.MAX_CHUNK_SIZE) { + log.warn("Invalid chunk size received: {}. Must be between {} and {}. Ignoring.", + requestedChunkSize, Constants.MIN_CHUNK_SIZE, Constants.MAX_CHUNK_SIZE); + // Do not set the invalid chunk size - keep the existing one + return; + } + // set chunk size on the connection RTMP state = conn.getState(); // set only the read chunk size since it came from the client diff --git a/common/src/main/java/org/red5/server/net/rtmp/codec/RTMPProtocolDecoder.java b/common/src/main/java/org/red5/server/net/rtmp/codec/RTMPProtocolDecoder.java index a98660c45..aeed64b02 100644 --- a/common/src/main/java/org/red5/server/net/rtmp/codec/RTMPProtocolDecoder.java +++ b/common/src/main/java/org/red5/server/net/rtmp/codec/RTMPProtocolDecoder.java @@ -320,7 +320,16 @@ public Packet decodePacket(RTMPConnection conn, RTMPDecodeState state, IoBuffer packet.setMessage(message); if (message instanceof ChunkSize) { ChunkSize chunkSizeMsg = (ChunkSize) message; - rtmp.setReadChunkSize(chunkSizeMsg.getSize()); + int requestedChunkSize = chunkSizeMsg.getSize(); + + // Validate chunk size to prevent security vulnerabilities + if (requestedChunkSize < Constants.MIN_CHUNK_SIZE || requestedChunkSize > Constants.MAX_CHUNK_SIZE) { + log.warn("Invalid chunk size received: {}. Must be between {} and {}. Ignoring.", + requestedChunkSize, Constants.MIN_CHUNK_SIZE, Constants.MAX_CHUNK_SIZE); + // Do not set the invalid chunk size - keep the existing one + } else { + rtmp.setReadChunkSize(requestedChunkSize); + } } else if (message instanceof Abort) { log.debug("Abort packet detected"); // client is aborting a message, reset the packet because the next chunk will start a new packet diff --git a/common/src/main/java/org/red5/server/net/rtmp/codec/RTMPProtocolEncoder.java b/common/src/main/java/org/red5/server/net/rtmp/codec/RTMPProtocolEncoder.java index 42851d7d5..08d83a4df 100755 --- a/common/src/main/java/org/red5/server/net/rtmp/codec/RTMPProtocolEncoder.java +++ b/common/src/main/java/org/red5/server/net/rtmp/codec/RTMPProtocolEncoder.java @@ -130,7 +130,16 @@ public IoBuffer encodePacket(Packet packet) { IRTMPEvent message = packet.getMessage(); if (message instanceof ChunkSize) { ChunkSize chunkSizeMsg = (ChunkSize) message; - ((RTMPConnection) Red5.getConnectionLocal()).getState().setWriteChunkSize(chunkSizeMsg.getSize()); + int requestedChunkSize = chunkSizeMsg.getSize(); + + // Validate chunk size to prevent security vulnerabilities + if (requestedChunkSize < Constants.MIN_CHUNK_SIZE || requestedChunkSize > Constants.MAX_CHUNK_SIZE) { + log.warn("Invalid chunk size for encoding: {}. Must be between {} and {}. Ignoring.", + requestedChunkSize, Constants.MIN_CHUNK_SIZE, Constants.MAX_CHUNK_SIZE); + // Do not set the invalid chunk size - keep the existing one + } else { + ((RTMPConnection) Red5.getConnectionLocal()).getState().setWriteChunkSize(requestedChunkSize); + } } // normally the message is expected not to be dropped if (!dropMessage(channelId, message)) { diff --git a/common/src/main/java/org/red5/server/net/rtmp/message/Constants.java b/common/src/main/java/org/red5/server/net/rtmp/message/Constants.java index 7fc2e2c07..ae66e5d83 100644 --- a/common/src/main/java/org/red5/server/net/rtmp/message/Constants.java +++ b/common/src/main/java/org/red5/server/net/rtmp/message/Constants.java @@ -34,6 +34,16 @@ public interface Constants { */ public static final byte TYPE_CHUNK_SIZE = 0x01; + /** + * Minimum allowed chunk size (1 byte) + */ + public static final int MIN_CHUNK_SIZE = 1; + + /** + * Maximum allowed chunk size (16MB per RTMP specification) + */ + public static final int MAX_CHUNK_SIZE = MEDIUM_INT_MAX; + /** * Abort message */ diff --git a/tests/src/test/java/org/red5/server/net/rtmp/codec/RTMPChunkSecurityTest.java b/tests/src/test/java/org/red5/server/net/rtmp/codec/RTMPChunkSecurityTest.java new file mode 100644 index 000000000..2760c9f5a --- /dev/null +++ b/tests/src/test/java/org/red5/server/net/rtmp/codec/RTMPChunkSecurityTest.java @@ -0,0 +1,143 @@ +package org.red5.server.net.rtmp.codec; + +import static org.junit.Assert.*; + +import org.apache.mina.core.buffer.IoBuffer; +import org.junit.Test; +import org.red5.server.net.rtmp.RTMPConnection; +import org.red5.server.net.rtmp.RTMPMinaConnection; +import org.red5.server.net.rtmp.message.Constants; + +/** + * Security tests for RTMP chunking vulnerabilities + */ +public class RTMPChunkSecurityTest { + + /** + * Test that large chunk sizes are properly validated and rejected + */ + @Test + public void testLargeChunkSizeValidation() { + RTMPConnection conn = new RTMPMinaConnection(); + conn.getState().setState(RTMP.STATE_CONNECTED); + + int initialChunkSize = conn.getState().getReadChunkSize(); + int maliciousChunkSize = Integer.MAX_VALUE; // 2GB+ chunk size + + // Create a ChunkSize event and try to set it + org.red5.server.net.rtmp.event.ChunkSize chunkSizeEvent = + new org.red5.server.net.rtmp.event.ChunkSize(maliciousChunkSize); + + // Simulate what happens in RTMPProtocolDecoder when processing chunk size + if (maliciousChunkSize < Constants.MIN_CHUNK_SIZE || maliciousChunkSize > Constants.MAX_CHUNK_SIZE) { + // Should NOT set the malicious chunk size + // Chunk size should remain unchanged + } else { + conn.getState().setReadChunkSize(maliciousChunkSize); + } + + // Verify that the chunk size was not changed to the malicious value + int currentChunkSize = conn.getState().getReadChunkSize(); + assertEquals("Malicious large chunk size should be rejected", initialChunkSize, currentChunkSize); + assertTrue("Chunk size should remain within valid bounds", + currentChunkSize >= Constants.MIN_CHUNK_SIZE && currentChunkSize <= Constants.MAX_CHUNK_SIZE); + } + + /** + * Test that negative chunk sizes are properly validated and rejected + */ + @Test + public void testNegativeChunkSizeValidation() { + RTMPConnection conn = new RTMPMinaConnection(); + conn.getState().setState(RTMP.STATE_CONNECTED); + + int initialChunkSize = conn.getState().getReadChunkSize(); + int negativeChunkSize = -1000; + + // Simulate the validation logic + if (negativeChunkSize < Constants.MIN_CHUNK_SIZE || negativeChunkSize > Constants.MAX_CHUNK_SIZE) { + // Should NOT set the negative chunk size + } else { + conn.getState().setReadChunkSize(negativeChunkSize); + } + + int currentChunkSize = conn.getState().getReadChunkSize(); + assertEquals("Negative chunk size should be rejected", initialChunkSize, currentChunkSize); + assertTrue("Chunk size should be positive", currentChunkSize > 0); + } + + /** + * Test that zero chunk sizes are properly validated and rejected + */ + @Test + public void testZeroChunkSizeValidation() { + RTMPConnection conn = new RTMPMinaConnection(); + conn.getState().setState(RTMP.STATE_CONNECTED); + + int initialChunkSize = conn.getState().getReadChunkSize(); + int zeroChunkSize = 0; + + // Simulate the validation logic + if (zeroChunkSize < Constants.MIN_CHUNK_SIZE || zeroChunkSize > Constants.MAX_CHUNK_SIZE) { + // Should NOT set the zero chunk size + } else { + conn.getState().setReadChunkSize(zeroChunkSize); + } + + int currentChunkSize = conn.getState().getReadChunkSize(); + assertEquals("Zero chunk size should be rejected", initialChunkSize, currentChunkSize); + assertTrue("Chunk size should be positive", currentChunkSize > 0); + } + + /** + * Test that valid chunk sizes are accepted + */ + @Test + public void testValidChunkSizeAccepted() { + RTMPConnection conn = new RTMPMinaConnection(); + conn.getState().setState(RTMP.STATE_CONNECTED); + + int validChunkSize = 4096; // 4KB, a reasonable chunk size + + // Simulate the validation logic + if (validChunkSize >= Constants.MIN_CHUNK_SIZE && validChunkSize <= Constants.MAX_CHUNK_SIZE) { + conn.getState().setReadChunkSize(validChunkSize); + } + + int currentChunkSize = conn.getState().getReadChunkSize(); + assertEquals("Valid chunk size should be accepted", validChunkSize, currentChunkSize); + } + + /** + * Test the boundary values for chunk size validation + */ + @Test + public void testChunkSizeBoundaryValues() { + RTMPConnection conn = new RTMPMinaConnection(); + conn.getState().setState(RTMP.STATE_CONNECTED); + + // Test minimum valid chunk size + int minValidSize = Constants.MIN_CHUNK_SIZE; + if (minValidSize >= Constants.MIN_CHUNK_SIZE && minValidSize <= Constants.MAX_CHUNK_SIZE) { + conn.getState().setReadChunkSize(minValidSize); + } + assertEquals("Minimum valid chunk size should be accepted", minValidSize, conn.getState().getReadChunkSize()); + + // Test maximum valid chunk size + conn.getState().setReadChunkSize(128); // Reset + int maxValidSize = Constants.MAX_CHUNK_SIZE; + if (maxValidSize >= Constants.MIN_CHUNK_SIZE && maxValidSize <= Constants.MAX_CHUNK_SIZE) { + conn.getState().setReadChunkSize(maxValidSize); + } + assertEquals("Maximum valid chunk size should be accepted", maxValidSize, conn.getState().getReadChunkSize()); + + // Test just over maximum (should be rejected) + conn.getState().setReadChunkSize(128); // Reset + int initialSize = conn.getState().getReadChunkSize(); + int overMaxSize = Constants.MAX_CHUNK_SIZE + 1; + if (overMaxSize >= Constants.MIN_CHUNK_SIZE && overMaxSize <= Constants.MAX_CHUNK_SIZE) { + conn.getState().setReadChunkSize(overMaxSize); + } + assertEquals("Over-maximum chunk size should be rejected", initialSize, conn.getState().getReadChunkSize()); + } +} \ No newline at end of file