Fix BZ 64563 - additional payload length validation

https://bz.apache.org/bugzilla/show_bug.cgi?id=64563
apache · Jun 30, 2020 · f9f75c1 · f9f75c1 · Beyond-My · Aug 7, 2020
1 parent 3d23880
commit f9f75c1
Show file tree

Hide file tree

Showing 3 changed files with 12 additions and 0 deletions.
diff --git a/java/org/apache/tomcat/websocket/LocalStrings.properties b/java/org/apache/tomcat/websocket/LocalStrings.properties
@@ -64,6 +64,7 @@ wsFrame.noContinuation=A new message was started when a continuation frame was e
 wsFrame.notMasked=The client frame was not masked but all client frames must be masked
 wsFrame.oneByteCloseCode=The client sent a close frame with a single byte payload which is not valid
 wsFrame.partialHeaderComplete=WebSocket frame received. fin [{0}], rsv [{1}], OpCode [{2}], payload length [{3}]
+wsFrame.payloadMsbInvalid=An invalid WebSocket frame was received - the most significant bit of a 64-bit payload was illegally set
 wsFrame.sessionClosed=The client data cannot be processed because the session has already been closed
 wsFrame.textMessageTooBig=The decoded text message was too big for the output buffer and the endpoint does not support partial messages
 wsFrame.wrongRsv=The client frame set the reserved bits to [{0}] for a message with opCode [{1}] which was not supported by this endpoint

diff --git a/java/org/apache/tomcat/websocket/WsFrameBase.java b/java/org/apache/tomcat/websocket/WsFrameBase.java
@@ -256,6 +256,13 @@ private boolean processRemainingHeader() throws IOException {
             readPos += 2;
         } else if (payloadLength == 127) {
             payloadLength = byteArrayToLong(inputBuffer, readPos, 8);
+            // The most significant bit of those 8 bytes is required to be zero
+            // (see RFC 6455, section 5.2). If the most significant bit is set,
+            // the resulting payload length will be negative so test for that.
+            if (payloadLength < 0) {
+                throw new WsIOException(
+                        new CloseReason(CloseCodes.PROTOCOL_ERROR, sm.getString("wsFrame.payloadMsbInvalid")));
+            }
             readPos += 8;
         }
         if (Util.isControl(opCode)) {

diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml
@@ -105,6 +105,10 @@
         Include the target URL in the log message when a WebSocket connection
         fails. (markt)
       </add>
+      <fix>
+        <bug>64563</bug>: Add additional validation of payload length for
+        WebSocket messages. (markt)
+      </fix>
     </changelog>
   </subsection>
   <subsection name="Other">