UNDERTOW-1395: ALPNLimitingSSLEngine validates buffer size.

Unnecessary to throw and catch an exception when we can validate content length first.
undertow-io · Aug 10, 2018 · 4501633 · 4501633
1 parent 2c7a2da
commit 4501633
Show file tree

Hide file tree

Showing 2 changed files with 15 additions and 3 deletions.
diff --git a/core/src/main/java/io/undertow/server/protocol/http/ALPNLimitingSSLEngine.java b/core/src/main/java/io/undertow/server/protocol/http/ALPNLimitingSSLEngine.java
@@ -37,6 +37,8 @@
  * @author Stuart Douglas
  */
 public class ALPNLimitingSSLEngine extends SSLEngine {
+    private static final SSLEngineResult UNDERFLOW_RESULT = new SSLEngineResult(
+            SSLEngineResult.Status.BUFFER_UNDERFLOW, SSLEngineResult.HandshakeStatus.NEED_UNWRAP, 0, 0);
 
     private final SSLEngine delegate;
     private final Runnable invalidAlpnRunnable;
@@ -72,6 +74,9 @@ public SSLEngineResult unwrap(ByteBuffer src, ByteBuffer dst) throws SSLExceptio
         if (done) {
             return delegate.unwrap(src, dst);
         }
+        if (ALPNOfferedClientHelloExplorer.isIncompleteHeader(src)) {
+            return UNDERFLOW_RESULT;
+        }
         try {
             List<Integer> clientCiphers = ALPNOfferedClientHelloExplorer.parseClientHello(src);
             if (clientCiphers != null) {
@@ -81,7 +86,7 @@ public SSLEngineResult unwrap(ByteBuffer src, ByteBuffer dst) throws SSLExceptio
                 done = true;
             }
         } catch (BufferUnderflowException e) {
-            return new SSLEngineResult(SSLEngineResult.Status.BUFFER_UNDERFLOW, SSLEngineResult.HandshakeStatus.NEED_UNWRAP, 0, 0);
+            return UNDERFLOW_RESULT;
         }
         return delegate.unwrap(src, dst);
     }
@@ -137,6 +142,9 @@ public SSLEngineResult unwrap(ByteBuffer byteBuffer, ByteBuffer[] byteBuffers, i
             return delegate.unwrap(byteBuffer, byteBuffers, i, i1);
         }
 
+        if (ALPNOfferedClientHelloExplorer.isIncompleteHeader(byteBuffer)) {
+            return UNDERFLOW_RESULT;
+        }
         try {
             List<Integer> clientCiphers = ALPNOfferedClientHelloExplorer.parseClientHello(byteBuffer);
             if (clientCiphers != null) {
@@ -146,7 +154,7 @@ public SSLEngineResult unwrap(ByteBuffer byteBuffer, ByteBuffer[] byteBuffers, i
                 done = true;
             }
         } catch (BufferUnderflowException e) {
-            return new SSLEngineResult(SSLEngineResult.Status.BUFFER_UNDERFLOW, SSLEngineResult.HandshakeStatus.NEED_UNWRAP, 0, 0);
+            return UNDERFLOW_RESULT;
         }
         return delegate.unwrap(byteBuffer, byteBuffers, i, i1);
     }

diff --git a/core/src/main/java/io/undertow/server/protocol/http/ALPNOfferedClientHelloExplorer.java b/core/src/main/java/io/undertow/server/protocol/http/ALPNOfferedClientHelloExplorer.java
@@ -42,6 +42,10 @@ private ALPNOfferedClientHelloExplorer() {
      */
     private static final int RECORD_HEADER_SIZE = 0x05;
 
+    static boolean isIncompleteHeader(ByteBuffer source) {
+        return source.remaining() < RECORD_HEADER_SIZE;
+    }
+
     /**
      * Checks if a client handshake is offering ALPN, and if so it returns a list of all ciphers. If ALPN is not being
      * offered then this will return null.
@@ -52,7 +56,7 @@ static List<Integer> parseClientHello(ByteBuffer source)
         ByteBuffer input = source.duplicate();
 
         // Do we have a complete header?
-        if (input.remaining() < RECORD_HEADER_SIZE) {
+        if (isIncompleteHeader(input)) {
             throw new BufferUnderflowException();
         }
         // Is it a handshake message?