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:05:39 UTC
[tomcat] branch 8.5.x updated (3003d7b -> e7c3817)
This is an automated email from the ASF dual-hosted git repository.
markt pushed a change to branch 8.5.x
in repository https://gitbox.apache.org/repos/asf/tomcat.git.
from 3003d7b Correct order
new 12d7156 Fix BZ 64563 - additional payload length validation
new e7c3817 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] 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 8.5.x
in repository https://gitbox.apache.org/repos/asf/tomcat.git
commit e7c3817bfbc45bf5315e11f9a593d26d791d7fa9
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 4afad67..a6df700 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 d2de204..64545f3 100644
--- a/webapps/docs/changelog.xml
+++ b/webapps/docs/changelog.xml
@@ -129,6 +129,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 8.5.x
in repository https://gitbox.apache.org/repos/asf/tomcat.git
commit 12d715676038efbf9c728af10163f8277fc019d5
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 28cdc30..4afad67 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 1ba0273..d2de204 100644
--- a/webapps/docs/changelog.xml
+++ b/webapps/docs/changelog.xml
@@ -123,6 +123,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="Other">
<changelog>
<fix>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org