You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tomcat.apache.org by ma...@apache.org on 2020/06/30 13:14:02 UTC

[tomcat] branch 7.0.x updated (3d23880 -> 34d19fb)

This is an automated email from the ASF dual-hosted git repository.

markt pushed a change to branch 7.0.x
in repository https://gitbox.apache.org/repos/asf/tomcat.git.


    from 3d23880  Align Chinese translations with 8.5.x
     new f9f75c1  Fix BZ 64563 - additional payload length validation
     new 34d19fb  Correct calculation of payload length when using 4 or more bytes

The 2 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails.  The revisions
listed as "add" were already present in the repository and have only
been added to this reference.


Summary of changes:
 .../tomcat/websocket/LocalStrings.properties       |  1 +
 java/org/apache/tomcat/websocket/WsFrameBase.java  |  9 +++++-
 test/org/apache/tomcat/websocket/TestWsFrame.java  | 32 ++++++++++++++++------
 webapps/docs/changelog.xml                         |  8 ++++++
 4 files changed, 40 insertions(+), 10 deletions(-)


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


[tomcat] 02/02: Correct calculation of payload length when using 4 or more bytes

Posted by ma...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

markt pushed a commit to branch 7.0.x
in repository https://gitbox.apache.org/repos/asf/tomcat.git

commit 34d19fbe243a6c84cac75c83118d3a0351f5ab41
Author: Mark Thomas <ma...@apache.org>
AuthorDate: Mon Jun 29 15:08:25 2020 +0100

    Correct calculation of payload length when using 4 or more bytes
---
 java/org/apache/tomcat/websocket/WsFrameBase.java |  2 +-
 test/org/apache/tomcat/websocket/TestWsFrame.java | 32 ++++++++++++++++-------
 webapps/docs/changelog.xml                        |  4 +++
 3 files changed, 28 insertions(+), 10 deletions(-)

diff --git a/java/org/apache/tomcat/websocket/WsFrameBase.java b/java/org/apache/tomcat/websocket/WsFrameBase.java
index e7b13fd..1d9404e 100644
--- a/java/org/apache/tomcat/websocket/WsFrameBase.java
+++ b/java/org/apache/tomcat/websocket/WsFrameBase.java
@@ -679,7 +679,7 @@ public abstract class WsFrameBase {
         int shift = 0;
         long result = 0;
         for (int i = start + len - 1; i >= start; i--) {
-            result = result + ((b[i] & 0xFF) << shift);
+            result = result + ((b[i] & 0xFFL) << shift);
             shift += 8;
         }
         return result;
diff --git a/test/org/apache/tomcat/websocket/TestWsFrame.java b/test/org/apache/tomcat/websocket/TestWsFrame.java
index 0b423ad..d6afce0 100644
--- a/test/org/apache/tomcat/websocket/TestWsFrame.java
+++ b/test/org/apache/tomcat/websocket/TestWsFrame.java
@@ -28,10 +28,17 @@ public class TestWsFrame {
         Assert.assertEquals(0L, WsFrameBase.byteArrayToLong(new byte[] { 0 }, 0, 1));
         Assert.assertEquals(1L, WsFrameBase.byteArrayToLong(new byte[] { 1 }, 0, 1));
         Assert.assertEquals(0xFF, WsFrameBase.byteArrayToLong(new byte[] { -1 }, 0, 1));
-        Assert.assertEquals(0xFFFF,
-                WsFrameBase.byteArrayToLong(new byte[] { -1, -1 }, 0, 2));
-        Assert.assertEquals(0xFFFFFF,
-                WsFrameBase.byteArrayToLong(new byte[] { -1, -1, -1 }, 0, 3));
+        Assert.assertEquals(0xFFFF, WsFrameBase.byteArrayToLong(new byte[] { -1, -1 }, 0, 2));
+        Assert.assertEquals(0xFFFFFF, WsFrameBase.byteArrayToLong(new byte[] { -1, -1, -1 }, 0, 3));
+        Assert.assertEquals(0xFFFFFFFFL, WsFrameBase.byteArrayToLong(new byte[] { -1, -1, -1, -1 }, 0, 4));
+        Assert.assertEquals(0xFFFFFFFFFFL, WsFrameBase.byteArrayToLong(new byte[] { -1, -1, -1, -1, -1 }, 0, 5));
+        Assert.assertEquals(0xFFFFFFFFFFFFL, WsFrameBase.byteArrayToLong(new byte[] { -1, -1, -1, -1, -1, -1 }, 0, 6));
+        Assert.assertEquals(0xFFFFFFFFFFFFFFL,
+                WsFrameBase.byteArrayToLong(new byte[] { -1, -1, -1, -1, -1, -1, -1 }, 0, 7));
+        Assert.assertEquals(0x7FFFFFFFFFFFFFFFL,
+                WsFrameBase.byteArrayToLong(new byte[] { 127, -1, -1, -1, -1, -1, -1, -1 }, 0, 8));
+        Assert.assertEquals(-1,
+                WsFrameBase.byteArrayToLong(new byte[] { -1, -1, -1, -1, -1, -1, -1, -1 }, 0, 8));
     }
 
 
@@ -40,10 +47,17 @@ public class TestWsFrame {
         Assert.assertEquals(0L, WsFrameBase.byteArrayToLong(new byte[] { 20, 0 }, 1, 1));
         Assert.assertEquals(1L, WsFrameBase.byteArrayToLong(new byte[] { 20, 1 }, 1, 1));
         Assert.assertEquals(0xFF, WsFrameBase.byteArrayToLong(new byte[] { 20, -1 }, 1, 1));
-        Assert.assertEquals(0xFFFF,
-                WsFrameBase.byteArrayToLong(new byte[] { 20, -1, -1 }, 1, 2));
-        Assert.assertEquals(0xFFFFFF,
-                WsFrameBase.byteArrayToLong(new byte[] { 20, -1, -1, -1 }, 1, 3));
+        Assert.assertEquals(0xFFFF, WsFrameBase.byteArrayToLong(new byte[] { 20, -1, -1 }, 1, 2));
+        Assert.assertEquals(0xFFFFFF, WsFrameBase.byteArrayToLong(new byte[] { 20, -1, -1, -1 }, 1, 3));
+        Assert.assertEquals(0xFFFFFFFFL, WsFrameBase.byteArrayToLong(new byte[] { 20, -1, -1, -1, -1 }, 1, 4));
+        Assert.assertEquals(0xFFFFFFFFFFL, WsFrameBase.byteArrayToLong(new byte[] { 20, -1, -1, -1, -1, -1 }, 1, 5));
+        Assert.assertEquals(0xFFFFFFFFFFFFL,
+                WsFrameBase.byteArrayToLong(new byte[] { 20, -1, -1, -1, -1, -1, -1 }, 1, 6));
+        Assert.assertEquals(0xFFFFFFFFFFFFFFL,
+                WsFrameBase.byteArrayToLong(new byte[] { 20, -1, -1, -1, -1, -1, -1, -1 }, 1, 7));
+        Assert.assertEquals(0x7FFFFFFFFFFFFFFFL,
+                WsFrameBase.byteArrayToLong(new byte[] { 20, 127, -1, -1, -1, -1, -1, -1, -1 }, 1, 8));
+        Assert.assertEquals(-1,
+                WsFrameBase.byteArrayToLong(new byte[] { 20, -1, -1, -1, -1, -1, -1, -1, -1 }, 1, 8));
     }
-
 }
diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml
index 93823de..c970cd8 100644
--- a/webapps/docs/changelog.xml
+++ b/webapps/docs/changelog.xml
@@ -109,6 +109,10 @@
         <bug>64563</bug>: Add additional validation of payload length for
         WebSocket messages. (markt)
       </fix>
+      <fix>
+        Correct the calculation of payload length when four or more bytes are
+        required to represent the payload length. (markt)
+      </fix>
     </changelog>
   </subsection>
   <subsection name="Other">


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


[tomcat] 01/02: Fix BZ 64563 - additional payload length validation

Posted by ma...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

markt pushed a commit to branch 7.0.x
in repository https://gitbox.apache.org/repos/asf/tomcat.git

commit f9f75c14678b68633f79030ddf4ff827f014cc84
Author: Mark Thomas <ma...@apache.org>
AuthorDate: Mon Jun 29 14:02:59 2020 +0100

    Fix BZ 64563 - additional payload length validation
    
    https://bz.apache.org/bugzilla/show_bug.cgi?id=64563
---
 java/org/apache/tomcat/websocket/LocalStrings.properties | 1 +
 java/org/apache/tomcat/websocket/WsFrameBase.java        | 7 +++++++
 webapps/docs/changelog.xml                               | 4 ++++
 3 files changed, 12 insertions(+)

diff --git a/java/org/apache/tomcat/websocket/LocalStrings.properties b/java/org/apache/tomcat/websocket/LocalStrings.properties
index 00a409c..0659225 100644
--- 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
index b3d7261..e7b13fd 100644
--- a/java/org/apache/tomcat/websocket/WsFrameBase.java
+++ b/java/org/apache/tomcat/websocket/WsFrameBase.java
@@ -256,6 +256,13 @@ public abstract class WsFrameBase {
             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
index 23df812..93823de 100644
--- 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">


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org