Jens Geyer created THRIFT-6180:
----------------------------------
Summary: C++ WebSocket server mis-frames empty and control frames
Key: THRIFT-6180
URL: https://issues.apache.org/jira/browse/THRIFT-6180
Project: Thrift
Issue Type: Bug
Components: C++ - Library
Reporter: Jens Geyer
Fix For: 0.25.0
TWebSocketServer gets WebSocket framing wrong in three related places, all in
lib/cpp/src/thrift/transport/TWebSocketServer.h. All three are demonstrated
below against master.
1. The masking key of a zero-length masked frame is never consumed.
readFrame reads the four masking-key bytes inside "if (length > 0)". RFC 6455
section 5.2 puts the masking key in every frame whose MASK bit is set, whatever
the payload length, and readFrame itself refuses any client frame that does not
set MASK. So for a zero-length frame the four bytes stay in the stream and the
next frame header is parsed out of the masking key instead.
A masking key is arbitrary, so this fails in whatever way the key happens to
encode. With the usual 0x37373737 the connection dies at once:
client: 89 80 37 37 37 37 (an empty Ping)
82 85 37 37 37 37 ... (a five-byte data frame)
server: TTransportException: Reserved bits must be zeroes
An empty Ping, an empty Pong and a Close with no status code are all ordinary
traffic that browsers send, so this is reachable without anything unusual on
the wire.
2. A Pong declares a length of zero and then writes the payload after it.
writeFrameHeader takes the length from writeBuffer_.available_read(), which
describes the body only for flush(), the one caller whose body is writeBuffer_.
pong() writes readBuffer_ instead:
ping of 8 bytes "PINGDATA" -> server writes 8A 00 50 49 4E 47 44 41 54 41
Opcode 0x8A is FIN|Pong, the declared length is 0, and eight bytes of payload
follow it. A conforming client reads a zero-length Pong and then parses 0x50 as
the next frame header, whose reserved bits are set, and must fail the
connection.
3. A Close frame does the same.
failConnection writes the two-byte close code after a header that declares zero:
server writes 88 00 03 F1 (0x03F1 = 1009, Message Too Big)
So the reason the connection was closed is unreadable to a conforming peer.
The fix gives writeFrameHeader the length it is describing rather than letting
it guess, so pong() and failConnection() declare what they actually write, and
moves the masking-key read out of the payload-length branch. A frame with no
payload also resets the read buffer, so that a Pong for an empty Ping echoes
nothing rather than whatever the previous frame left unread.
Tests in lib/cpp/test/TWebSocketServerTest.cpp cover all three: an empty masked
Ping followed by a data frame is read normally, a Pong declares the length it
carries, and a Close declares its two bytes.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
