From: Markus Koschany <apo@debian.org>
Date: Sun, 5 Nov 2023 21:05:39 +0100
Subject: CVE-2023-34462

Bug-Debian: https://bugs.debian.org/1038947
Origin: https://github.com/netty/netty/commit/535da17e45201ae4278c0479e6162bb4127d4c32
---
 .../java/io/netty/util/internal/ObjectUtil.java    | 55 ++++++++++++++++++++--
 .../io/netty/handler/ssl/AbstractSniHandler.java   | 22 +++++++++
 .../main/java/io/netty/handler/ssl/SniHandler.java | 50 ++++++++++++++++++++
 .../netty/handler/ssl/SslClientHelloHandler.java   | 32 +++++++++++++
 4 files changed, 156 insertions(+), 3 deletions(-)

diff --git a/common/src/main/java/io/netty/util/internal/ObjectUtil.java b/common/src/main/java/io/netty/util/internal/ObjectUtil.java
index cbac561..ef26046 100644
--- a/common/src/main/java/io/netty/util/internal/ObjectUtil.java
+++ b/common/src/main/java/io/netty/util/internal/ObjectUtil.java
@@ -21,6 +21,11 @@ import java.util.Collection;
  */
 public final class ObjectUtil {
 
+    private static final float FLOAT_ZERO = 0.0F;
+    private static final double DOUBLE_ZERO = 0.0D;
+    private static final long LONG_ZERO = 0L;
+    private static final int INT_ZERO = 0;
+
     private ObjectUtil() {
     }
 
@@ -72,13 +77,57 @@ public final class ObjectUtil {
      * Checks that the given argument is positive or zero. If it is not, throws {@link IllegalArgumentException}.
      * Otherwise, returns the argument.
      */
-    public static long checkPositiveOrZero(long i, String name) {
-        if (i < 0) {
-            throw new IllegalArgumentException(name + ": " + i + " (expected: >= 0)");
+    public static long checkPositiveOrZero(long l, String name) {
+        if (l < 0) {
+            throw new IllegalArgumentException(name + ": " + l + " (expected: >= 0)");
+        }
+        return l;
+    }
+
+     /**
+     * Checks that the given argument is positive or zero. If it is not, throws {@link IllegalArgumentException}.
+     * Otherwise, returns the argument.
+     */
+    public static double checkPositiveOrZero(final double d, final String name) {
+        if (d < DOUBLE_ZERO) {
+            throw new IllegalArgumentException(name + " : " + d + " (expected: >= 0)");
+        }
+        return d;
+    }
+
+    /**
+     * Checks that the given argument is positive or zero. If it is not, throws {@link IllegalArgumentException}.
+     * Otherwise, returns the argument.
+     */
+    public static float checkPositiveOrZero(final float f, final String name) {
+        if (f < FLOAT_ZERO) {
+            throw new IllegalArgumentException(name + " : " + f + " (expected: >= 0)");
+        }
+        return f;
+    }
+
+    /**
+     * Checks that the given argument is in range. If it is not, throws {@link IllegalArgumentException}.
+     * Otherwise, returns the argument.
+     */
+    public static int checkInRange(int i, int start, int end, String name) {
+        if (i < start || i > end) {
+            throw new IllegalArgumentException(name + ": " + i + " (expected: " + start + "-" + end + ")");
         }
         return i;
     }
 
+    /**
+     * Checks that the given argument is in range. If it is not, throws {@link IllegalArgumentException}.
+     * Otherwise, returns the argument.
+     */
+    public static long checkInRange(long l, long start, long end, String name) {
+        if (l < start || l > end) {
+            throw new IllegalArgumentException(name + ": " + l + " (expected: " + start + "-" + end + ")");
+        }
+        return l;
+    }
+
     /**
      * Checks that the given argument is neither null nor empty.
      * If it is, throws {@link NullPointerException} or {@link IllegalArgumentException}.
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 dcba585..02baad0 100644
--- a/handler/src/main/java/io/netty/handler/ssl/AbstractSniHandler.java
+++ b/handler/src/main/java/io/netty/handler/ssl/AbstractSniHandler.java
@@ -19,6 +19,7 @@ import io.netty.buffer.ByteBuf;
 import io.netty.channel.ChannelHandlerContext;
 import io.netty.util.CharsetUtil;
 import io.netty.util.concurrent.Future;
+import static io.netty.util.internal.ObjectUtil.checkPositiveOrZero;
 
 import java.util.Locale;
 
@@ -117,8 +118,29 @@ public abstract class AbstractSniHandler<T> extends SslClientHelloHandler<T> {
         return null;
     }
 
+    protected final long handshakeTimeoutMillis;
     private String hostname;
 
+    /**
+     * @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(0, 0L);
+    }
+
     @Override
     protected Future<T> lookup(ChannelHandlerContext ctx, ByteBuf clientHello) throws Exception {
         hostname = clientHello == null ? null : extractSniHostname(clientHello);
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 c6a8227..f5e9589 100644
--- a/handler/src/main/java/io/netty/handler/ssl/SniHandler.java
+++ b/handler/src/main/java/io/netty/handler/ssl/SniHandler.java
@@ -51,6 +51,19 @@ public class SniHandler extends AbstractSniHandler<SslContext> {
         this(new AsyncMappingAdapter(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 maxClientHelloLength the maximum length of the client hello message
+     * @param handshakeTimeoutMillis the handshake timeout in milliseconds
+     */
+    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}
@@ -69,9 +82,46 @@ public class SniHandler extends AbstractSniHandler<SslContext> {
      */
     @SuppressWarnings("unchecked")
     public SniHandler(AsyncMapping<? super String, ? extends SslContext> mapping) {
+        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,
+                      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
      */
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 4bcf349..8ef0069 100644
--- a/handler/src/main/java/io/netty/handler/ssl/SslClientHelloHandler.java
+++ b/handler/src/main/java/io/netty/handler/ssl/SslClientHelloHandler.java
@@ -22,8 +22,10 @@ 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;
@@ -36,14 +38,32 @@ import java.util.List;
  */
 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) {
@@ -117,6 +137,15 @@ public abstract class SslClientHelloHandler<T> extends ByteToMessageDecoder impl
                                     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;
@@ -161,6 +190,9 @@ 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()) {