CVE-2023-34462: Uncontrolled Resource Consumption (CWE-400)
value: Netty is an asynchronous event-driven network application framework for rapid development of maintainable high performance protocol servers & clients. The `SniHandler` can allocate up to 16MB of heap for each channel during the TLS handshake. When the handler or the channel does not have an idle timeout, it can be used to make a TCP server using the `SniHandler` to allocate 16MB of heap. The `SniHandler` class is a handler that waits for the TLS handshake to configure a `SslHandler` according to the indicated server name by the `ClientHello` record. For this matter it allocates a `ByteBuf` using the value defined in the `ClientHello` record. Normally the value of the packet should be smaller than the handshake packet but there are not checks done here and the way the code is written, it is possible to craft a packet that makes the `SslClientHelloHandler`. This vulnerability has been fixed in version 4.1.94.Final.
CWE description: The product does not properly control the allocation and maintenance of a limited resource, thereby enabling an actor to influence the amount of resources consumed, eventually leading to the exhaustion of available resources.
CVSS v3 base score: 6.5
Published: 2023-06-22T23:15Z
Repository: https://github.com/netty/netty
Language: Java
Files changed: handler/src/main/java/io/netty/handler/ssl/AbstractSniHandler.java, handler/src/main/java/io/netty/handler/ssl/SniHandler.java, handler/src/main/java/io/netty/handler/ssl/SslClientHelloHandler.java, handler/src/test/java/io/netty/handler/ssl/SniHandlerTest.java
Commit date: 2023-06-20 14:53:08 +0200
Commit message: Merge pull request from GHSA-6mjq-h674-j845

Motivation:

In theory the ClientHello can span multiple records and so reach the length of 16MB. This can result in high memory usage, we should allow the user to define a maximum length.

Modifications:

Add new constructor which allows to limit the maximum length of a ClientHello message.

Result:

Be able to guard against high memory usage when parsing ClientHello messages
Diff: diff --git a/handler/src/main/java/io/netty/handler/ssl/AbstractSniHandler.java b/handler/src/main/java/io/netty/handler/ssl/AbstractSniHandler.java
index 148dd6db7fa..5abf8a1b107 100644
--- a/handler/src/main/java/io/netty/handler/ssl/AbstractSniHandler.java
+++ b/handler/src/main/java/io/netty/handler/ssl/AbstractSniHandler.java
@@ -124,18 +124,27 @@ public abstract class AbstractSniHandler<T> extends SslClientHelloHandler<T> {
     protected final long handshakeTimeoutMillis;
     private ScheduledFuture<?> timeoutFuture;
     private String hostname;
 
     /**
-     * @param handshakeTimeoutMillis the handshake timeout in milliseconds
+     * @param handshakeTimeoutMillis    the handshake timeout in milliseconds
      */
     protected AbstractSniHandler(long handshakeTimeoutMillis) {
+        this(0, handshakeTimeoutMillis);
+    }
+
+    /**
+     * @paramm maxClientHelloLength     the maximum length of the client hello message.
+     * @param handshakeTimeoutMillis    the handshake timeout in milliseconds
+     */
+    protected AbstractSniHandler(int maxClientHelloLength, long handshakeTimeoutMillis) {
+        super(maxClientHelloLength);
         this.handshakeTimeoutMillis = checkPositiveOrZero(handshakeTimeoutMillis, "handshakeTimeoutMillis");
     }
 
     public AbstractSniHandler() {
-        this(0L);
+        this(0, 0L);
     }
 
     @Override
     public void handlerAdded(ChannelHandlerContext ctx) throws Exception {
         if (ctx.channel().isActive()) {
diff --git a/handler/src/main/java/io/netty/handler/ssl/SniHandler.java b/handler/src/main/java/io/netty/handler/ssl/SniHandler.java
index 09d40bf46ac..0f1d069c62a 100644
--- a/handler/src/main/java/io/netty/handler/ssl/SniHandler.java
+++ b/handler/src/main/java/io/netty/handler/ssl/SniHandler.java
@@ -54,14 +54,16 @@ public class SniHandler extends AbstractSniHandler<SslContext> {
     /**
      * Creates a SNI detection handler with configured {@link SslContext}
      * maintained by {@link Mapping}
      *
      * @param mapping the mapping of domain name to {@link SslContext}
+     * @param maxClientHelloLength the maximum length of the client hello message
      * @param handshakeTimeoutMillis the handshake timeout in milliseconds
      */
-    public SniHandler(Mapping<? super String, ? extends SslContext> mapping, long handshakeTimeoutMillis) {
-        this(new AsyncMappingAdapter(mapping), handshakeTimeoutMillis);
+    public SniHandler(Mapping<? super String, ? extends SslContext> mapping,
+                      int maxClientHelloLength, long handshakeTimeoutMillis) {
+        this(new AsyncMappingAdapter(mapping), maxClientHelloLength, handshakeTimeoutMillis);
     }
 
     /**
      * Creates a SNI detection handler with configured {@link SslContext}
      * maintained by {@link DomainNameMapping}
@@ -78,26 +80,50 @@ public class SniHandler extends AbstractSniHandler<SslContext> {
      *
      * @param mapping the mapping of domain name to {@link SslContext}
      */
     @SuppressWarnings("unchecked")
     public SniHandler(AsyncMapping<? super String, ? extends SslContext> mapping) {
-        this(mapping, 0L);
+        this(mapping, 0, 0L);
     }
 
     /**
      * Creates a SNI detection handler with configured {@link SslContext}
      * maintained by {@link AsyncMapping}
      *
      * @param mapping the mapping of domain name to {@link SslContext}
+     * @param maxClientHelloLength the maximum length of the client hello message
      * @param handshakeTimeoutMillis the handshake timeout in milliseconds
      */
     @SuppressWarnings("unchecked")
-    public SniHandler(AsyncMapping<? super String, ? extends SslContext> mapping, long handshakeTimeoutMillis) {
-        super(handshakeTimeoutMillis);
+    public SniHandler(AsyncMapping<? super String, ? extends SslContext> mapping,
+                      int maxClientHelloLength, long handshakeTimeoutMillis) {
+        super(maxClientHelloLength, handshakeTimeoutMillis);
         this.mapping = (AsyncMapping<String, SslContext>) ObjectUtil.checkNotNull(mapping, "mapping");
     }
 
+    /**
+     * Creates a SNI detection handler with configured {@link SslContext}
+     * maintained by {@link Mapping}
+     *
+     * @param mapping the mapping of domain name to {@link SslContext}
+     * @param handshakeTimeoutMillis the handshake timeout in milliseconds
+     */
+    public SniHandler(Mapping<? super String, ? extends SslContext> mapping, long handshakeTimeoutMillis) {
+        this(new AsyncMappingAdapter(mapping), handshakeTimeoutMillis);
+    }
+
+    /**
+     * Creates a SNI detection handler with configured {@link SslContext}
+     * maintained by {@link AsyncMapping}
+     *
+     * @param mapping the mapping of domain name to {@link SslContext}
+     * @param handshakeTimeoutMillis the handshake timeout in milliseconds
+     */
+    public SniHandler(AsyncMapping<? super String, ? extends SslContext> mapping, long handshakeTimeoutMillis) {
+        this(mapping, 0, handshakeTimeoutMillis);
+    }
+
     /**
      * @return the selected hostname
      */
     public String hostname() {
         return selection.hostname;
diff --git a/handler/src/main/java/io/netty/handler/ssl/SslClientHelloHandler.java b/handler/src/main/java/io/netty/handler/ssl/SslClientHelloHandler.java
index ee1cd388bdc..2cb3076afc1 100644
--- a/handler/src/main/java/io/netty/handler/ssl/SslClientHelloHandler.java
+++ b/handler/src/main/java/io/netty/handler/ssl/SslClientHelloHandler.java
@@ -20,12 +20,14 @@ import io.netty.buffer.ByteBufUtil;
 import io.netty.channel.ChannelHandlerContext;
 import io.netty.channel.ChannelOutboundHandler;
 import io.netty.channel.ChannelPromise;
 import io.netty.handler.codec.ByteToMessageDecoder;
 import io.netty.handler.codec.DecoderException;
+import io.netty.handler.codec.TooLongFrameException;
 import io.netty.util.concurrent.Future;
 import io.netty.util.concurrent.FutureListener;
+import io.netty.util.internal.ObjectUtil;
 import io.netty.util.internal.PlatformDependent;
 import io.netty.util.internal.logging.InternalLogger;
 import io.netty.util.internal.logging.InternalLoggerFactory;
 
 import java.net.SocketAddress;
@@ -34,18 +36,36 @@ import java.util.List;
 /**
  * {@link ByteToMessageDecoder} which allows to be notified once a full {@code ClientHello} was received.
  */
 public abstract class SslClientHelloHandler<T> extends ByteToMessageDecoder implements ChannelOutboundHandler {
 
+    /**
+     * The maximum length of client hello message as defined by
+     * <a href="https://www.rfc-editor.org/rfc/rfc5246#section-6.2.1">RFC5246</a>.
+     */
+    public static final int MAX_CLIENT_HELLO_LENGTH = 0xFFFFFF;
+
     private static final InternalLogger logger =
             InternalLoggerFactory.getInstance(SslClientHelloHandler.class);
 
+    private final int maxClientHelloLength;
     private boolean handshakeFailed;
     private boolean suppressRead;
     private boolean readPending;
     private ByteBuf handshakeBuffer;
 
+    public SslClientHelloHandler() {
+        this(MAX_CLIENT_HELLO_LENGTH);
+    }
+
+    protected SslClientHelloHandler(int maxClientHelloLength) {
+        // 16MB is the maximum as per RFC:
+        // See https://www.rfc-editor.org/rfc/rfc5246#section-6.2.1
+        this.maxClientHelloLength =
+                ObjectUtil.checkInRange(maxClientHelloLength, 0, MAX_CLIENT_HELLO_LENGTH, "maxClientHelloLength");
+    }
+
     @Override
     protected void decode(ChannelHandlerContext ctx, ByteBuf in, List<Object> out) throws Exception {
         if (!suppressRead && !handshakeFailed) {
             try {
                 int readerIndex = in.readerIndex();
@@ -115,10 +135,19 @@ public abstract class SslClientHelloHandler<T> extends ByteToMessageDecoder impl
                                     // Read the length of the handshake as it may arrive in fragments
                                     // See https://tools.ietf.org/html/rfc5246#section-7.4
                                     handshakeLength = in.getUnsignedMedium(readerIndex +
                                             SslUtils.SSL_RECORD_HEADER_LENGTH + 1);
 
+                                    if (handshakeLength > maxClientHelloLength && maxClientHelloLength != 0) {
+                                        TooLongFrameException e = new TooLongFrameException(
+                                                "ClientHello length exceeds " + maxClientHelloLength +
+                                                        ": " + handshakeLength);
+                                        in.skipBytes(in.readableBytes());
+                                        ctx.fireUserEventTriggered(new SniCompletionEvent(e));
+                                        SslUtils.handleHandshakeFailure(ctx, e, true);
+                                        throw e;
+                                    }
                                     // Consume handshakeType and handshakeLength (this sums up as 4 bytes)
                                     readerIndex += 4;
                                     packetLength -= 4;
 
                                     if (handshakeLength + 4 + SslUtils.SSL_RECORD_HEADER_LENGTH <= packetLength) {
@@ -159,10 +188,13 @@ public abstract class SslClientHelloHandler<T> extends ByteToMessageDecoder impl
                     }
                 }
             } catch (NotSslRecordException e) {
                 // Just rethrow as in this case we also closed the channel and this is consistent with SslHandler.
                 throw e;
+            } catch (TooLongFrameException e) {
+                // Just rethrow as in this case we also closed the channel
+                throw e;
             } catch (Exception e) {
                 // unexpected encoding, ignore sni and use default
                 if (logger.isDebugEnabled()) {
                     logger.debug("Unexpected client hello packet: " + ByteBufUtil.hexDump(in), e);
                 }
diff --git a/handler/src/test/java/io/netty/handler/ssl/SniHandlerTest.java b/handler/src/test/java/io/netty/handler/ssl/SniHandlerTest.java
index fa06b319368..25de5476228 100644
--- a/handler/src/test/java/io/netty/handler/ssl/SniHandlerTest.java
+++ b/handler/src/test/java/io/netty/handler/ssl/SniHandlerTest.java
@@ -23,15 +23,14 @@ import java.util.List;
 import java.util.concurrent.CountDownLatch;
 import java.util.concurrent.TimeUnit;
 import java.util.concurrent.atomic.AtomicBoolean;
 import java.util.concurrent.atomic.AtomicReference;
 
-import javax.net.ssl.HandshakeCompletedEvent;
-import javax.net.ssl.SSLContext;
 import javax.net.ssl.SSLEngine;
 import javax.net.ssl.SSLException;
 
+import io.netty.handler.codec.TooLongFrameException;
 import io.netty.util.concurrent.Future;
 
 import io.netty.bootstrap.Bootstrap;
 import io.netty.bootstrap.ServerBootstrap;
 import io.netty.buffer.ByteBuf;
@@ -713,18 +712,68 @@ public class SniHandlerTest {
         }
         clientHello.release();
         return result;
     }
 
+    @Test
+    public void testSniHandlerFailsOnTooBigClientHello() throws Exception {
+        SniHandler handler = new SniHandler(new Mapping<String, SslContext>() {
+            @Override
+            public SslContext map(String input) {
+                throw new UnsupportedOperationException("Should not be called");
+            }
+        }, 10, 0);
+
+        final AtomicReference<SniCompletionEvent> completionEventRef =
+                new AtomicReference<SniCompletionEvent>();
+        final EmbeddedChannel ch = new EmbeddedChannel(handler, new ChannelInboundHandlerAdapter() {
+            @Override
+            public void userEventTriggered(ChannelHandlerContext ctx, Object evt) {
+                if (evt instanceof SniCompletionEvent) {
+                    completionEventRef.set((SniCompletionEvent) evt);
+                }
+            }
+        });
+        final ByteBuf buffer = ch.alloc().buffer();
+        buffer.writeByte(0x16);      // Content Type: Handshake
+        buffer.writeShort((short) 0x0303); // TLS 1.2
+        buffer.writeShort((short) 0x0006); // Packet length
+
+        // 16_777_215
+        buffer.writeByte((byte) 0x01); // Client Hello
+        buffer.writeMedium(0xFFFFFF); // Length
+        buffer.writeShort((short) 0x0303); // TLS 1.2
+
+        assertThrows(TooLongFrameException.class, new Executable() {
+            @Override
+            public void execute() throws Throwable {
+                ch.writeInbound(buffer);
+            }
+        });
+        try {
+            while (completionEventRef.get() == null) {
+                Thread.sleep(100);
+                // We need to run all pending tasks as the handshake timeout is scheduled on the EventLoop.
+                ch.runPendingTasks();
+            }
+            SniCompletionEvent completionEvent = completionEventRef.get();
+            assertNotNull(completionEvent);
+            assertNotNull(completionEvent.cause());
+            assertEquals(TooLongFrameException.class, completionEvent.cause().getClass());
+        } finally {
+            ch.finishAndReleaseAll();
+        }
+    }
+
     @Test
     public void testSniHandlerFiresHandshakeTimeout() throws Exception {
         SniHandler handler = new SniHandler(new Mapping<String, SslContext>() {
             @Override
             public SslContext map(String input) {
                 throw new UnsupportedOperationException("Should not be called");
             }
-        }, 10);
+        }, 0, 10);
 
         final AtomicReference<SniCompletionEvent> completionEventRef =
             new AtomicReference<SniCompletionEvent>();
         EmbeddedChannel ch = new EmbeddedChannel(handler, new ChannelInboundHandlerAdapter() {
             @Override
@@ -756,11 +805,11 @@ public class SniHandlerTest {
         SniHandler handler = new SniHandler(new Mapping<String, SslContext>() {
             @Override
             public SslContext map(String input) {
                 return context;
             }
-        }, 100);
+        }, 0, 100);
 
         final AtomicReference<SniCompletionEvent> sniCompletionEventRef =
             new AtomicReference<SniCompletionEvent>();
         final AtomicReference<SslHandshakeCompletionEvent> handshakeCompletionEventRef =
             new AtomicReference<SslHandshakeCompletionEvent>();

Vulnerable code:      * @param handshakeTimeoutMillis the handshake timeout in milliseconds
        this(0L);
    public SniHandler(Mapping<? super String, ? extends SslContext> mapping, long handshakeTimeoutMillis) {
        this(new AsyncMappingAdapter(mapping), handshakeTimeoutMillis);
        this(mapping, 0L);
    public SniHandler(AsyncMapping<? super String, ? extends SslContext> mapping, long handshakeTimeoutMillis) {
        super(handshakeTimeoutMillis);
import javax.net.ssl.HandshakeCompletedEvent;
import javax.net.ssl.SSLContext;
        }, 10);
        }, 100);
Fixed code:      * @param handshakeTimeoutMillis    the handshake timeout in milliseconds
        this(0, handshakeTimeoutMillis);
    }

    /**
     * @paramm maxClientHelloLength     the maximum length of the client hello message.
     * @param handshakeTimeoutMillis    the handshake timeout in milliseconds
     */
    protected AbstractSniHandler(int maxClientHelloLength, long handshakeTimeoutMillis) {
        super(maxClientHelloLength);
        this(0, 0L);
     * @param maxClientHelloLength the maximum length of the client hello message
    public SniHandler(Mapping<? super String, ? extends SslContext> mapping,
                      int maxClientHelloLength, long handshakeTimeoutMillis) {
        this(new AsyncMappingAdapter(mapping), maxClientHelloLength, handshakeTimeoutMillis);
        this(mapping, 0, 0L);
     * @param maxClientHelloLength the maximum length of the client hello message
    public SniHandler(AsyncMapping<? super String, ? extends SslContext> mapping,
                      int maxClientHelloLength, long handshakeTimeoutMillis) {
        super(maxClientHelloLength, handshakeTimeoutMillis);
    /**
     * Creates a SNI detection handler with configured {@link SslContext}
     * maintained by {@link Mapping}
     *
     * @param mapping the mapping of domain name to {@link SslContext}
     * @param handshakeTimeoutMillis the handshake timeout in milliseconds
     */
    public SniHandler(Mapping<? super String, ? extends SslContext> mapping, long handshakeTimeoutMillis) {
        this(new AsyncMappingAdapter(mapping), handshakeTimeoutMillis);
    }

    /**
     * Creates a SNI detection handler with configured {@link SslContext}
     * maintained by {@link AsyncMapping}
     *
     * @param mapping the mapping of domain name to {@link SslContext}
     * @param handshakeTimeoutMillis the handshake timeout in milliseconds
     */
    public SniHandler(AsyncMapping<? super String, ? extends SslContext> mapping, long handshakeTimeoutMillis) {
        this(mapping, 0, handshakeTimeoutMillis);
    }

import io.netty.handler.codec.TooLongFrameException;
import io.netty.util.internal.ObjectUtil;
    /**
     * The maximum length of client hello message as defined by
     * <a href="https://www.rfc-editor.org/rfc/rfc5246#section-6.2.1">RFC5246</a>.
     */
    public static final int MAX_CLIENT_HELLO_LENGTH = 0xFFFFFF;

    private final int maxClientHelloLength;
    public SslClientHelloHandler() {
        this(MAX_CLIENT_HELLO_LENGTH);
    }

    protected SslClientHelloHandler(int maxClientHelloLength) {
        // 16MB is the maximum as per RFC:
        // See https://www.rfc-editor.org/rfc/rfc5246#section-6.2.1
        this.maxClientHelloLength =
                ObjectUtil.checkInRange(maxClientHelloLength, 0, MAX_CLIENT_HELLO_LENGTH, "maxClientHelloLength");
    }

                                    if (handshakeLength > maxClientHelloLength && maxClientHelloLength != 0) {
                                        TooLongFrameException e = new TooLongFrameException(
                                                "ClientHello length exceeds " + maxClientHelloLength +
                                                        ": " + handshakeLength);
                                        in.skipBytes(in.readableBytes());
                                        ctx.fireUserEventTriggered(new SniCompletionEvent(e));
                                        SslUtils.handleHandshakeFailure(ctx, e, true);
                                        throw e;
                                    }
            } catch (TooLongFrameException e) {
                // Just rethrow as in this case we also closed the channel
                throw e;
import io.netty.handler.codec.TooLongFrameException;
    @Test
    public void testSniHandlerFailsOnTooBigClientHello() throws Exception {
        SniHandler handler = new SniHandler(new Mapping<String, SslContext>() {
            @Override
            public SslContext map(String input) {
                throw new UnsupportedOperationException("Should not be called");
            }
        }, 10, 0);

        final AtomicReference<SniCompletionEvent> completionEventRef =
                new AtomicReference<SniCompletionEvent>();
        final EmbeddedChannel ch = new EmbeddedChannel(handler, new ChannelInboundHandlerAdapter() {
            @Override
            public void userEventTriggered(ChannelHandlerContext ctx, Object evt) {
                if (evt instanceof SniCompletionEvent) {
                    completionEventRef.set((SniCompletionEvent) evt);
                }
            }
        });
        final ByteBuf buffer = ch.alloc().buffer();
        buffer.writeByte(0x16);      // Content Type: Handshake
        buffer.writeShort((short) 0x0303); // TLS 1.2
        buffer.writeShort((short) 0x0006); // Packet length

        // 16_777_215
        buffer.writeByte((byte) 0x01); // Client Hello
        buffer.writeMedium(0xFFFFFF); // Length
        buffer.writeShort((short) 0x0303); // TLS 1.2

        assertThrows(TooLongFrameException.class, new Executable() {
            @Override
            public void execute() throws Throwable {
                ch.writeInbound(buffer);
            }
        });
        try {
            while (completionEventRef.get() == null) {
                Thread.sleep(100);
                // We need to run all pending tasks as the handshake timeout is scheduled on the EventLoop.
                ch.runPendingTasks();
            }
            SniCompletionEvent completionEvent = completionEventRef.get();
            assertNotNull(completionEvent);
            assertNotNull(completionEvent.cause());
            assertEquals(TooLongFrameException.class, completionEvent.cause().getClass());
        } finally {
            ch.finishAndReleaseAll();
        }
    }

        }, 0, 10);
        }, 0, 100);