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:03:10 UTC
[tomcat] branch master updated (f0d9610 -> 1cddae8)
This is an automated email from the ASF dual-hosted git repository.
markt pushed a change to branch master
in repository https://gitbox.apache.org/repos/asf/tomcat.git.
from f0d9610 Extend the list of keys known to differ between versions
new 1c1c77b Fix BZ 64563 - additional payload length validation
new 1cddae8 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 | 12 ++++++++
4 files changed, 44 insertions(+), 10 deletions(-)
---------------------------------------------------------------------
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 master
in repository https://gitbox.apache.org/repos/asf/tomcat.git
commit 1c1c77b0efb667cea80b532440b44cea1dc427c3
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 | 8 ++++++++
3 files changed, 16 insertions(+)
diff --git a/java/org/apache/tomcat/websocket/LocalStrings.properties b/java/org/apache/tomcat/websocket/LocalStrings.properties
index 9412ffe..929822d 100644
--- a/java/org/apache/tomcat/websocket/LocalStrings.properties
+++ b/java/org/apache/tomcat/websocket/LocalStrings.properties
@@ -71,6 +71,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.suspendRequested=Suspend of the message receiving has already been requested.
wsFrame.textMessageTooBig=The decoded text message was too big for the output buffer and the endpoint does not support partial messages
diff --git a/java/org/apache/tomcat/websocket/WsFrameBase.java b/java/org/apache/tomcat/websocket/WsFrameBase.java
index 97c9c94..ba5813b 100644
--- a/java/org/apache/tomcat/websocket/WsFrameBase.java
+++ b/java/org/apache/tomcat/websocket/WsFrameBase.java
@@ -261,6 +261,13 @@ public abstract class WsFrameBase {
} else if (payloadLength == 127) {
payloadLength = byteArrayToLong(inputBuffer.array(),
inputBuffer.arrayOffset() + inputBuffer.position(), 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")));
+ }
inputBuffer.position(inputBuffer.position() + 8);
}
if (Util.isControl(opCode)) {
diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml
index 58a3acd..4bc2181 100644
--- a/webapps/docs/changelog.xml
+++ b/webapps/docs/changelog.xml
@@ -138,6 +138,14 @@
</fix>
</changelog>
</subsection>
+ <subsection name="WebSocket">
+ <changelog>
+ <fix>
+ <bug>64563</bug>: Add additional validation of payload length for
+ WebSocket messages. (markt)
+ </fix>
+ </changelog>
+ </subsection>
<subsection name="Web Applications">
<changelog>
<update>
---------------------------------------------------------------------
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 master
in repository https://gitbox.apache.org/repos/asf/tomcat.git
commit 1cddae8da4ecb4ac04575d3b5fba2daa2e0c8ead
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 ba5813b..a5d2aaa 100644
--- a/java/org/apache/tomcat/websocket/WsFrameBase.java
+++ b/java/org/apache/tomcat/websocket/WsFrameBase.java
@@ -678,7 +678,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 4bc2181..1fbf038 100644
--- a/webapps/docs/changelog.xml
+++ b/webapps/docs/changelog.xml
@@ -144,6 +144,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="Web Applications">
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org