Jens Geyer created THRIFT-6177:
----------------------------------
Summary: Bound the WebSocket frame payload length before it sizes
the read buffer in the C++ library
Key: THRIFT-6177
URL: https://issues.apache.org/jira/browse/THRIFT-6177
Project: Thrift
Issue Type: Bug
Components: C++ - Library
Reporter: Jens Geyer
Fix For: 0.25.0
TWebSocketServer::readFrame (lib/cpp/src/thrift/transport/TWebSocketServer.h)
sizes its read buffer from the payload length the frame header declares, before
any payload byte has arrived:
// size_t is smaller than a ulong on a 32-bit system
if (payloadLength > UINT32_MAX) {
failConnection(CloseCode::MessageTooBig);
return false;
}
auto length = static_cast<uint32_t>(payloadLength);
if (length > 0) {
...
readBuffer_.resetBuffer(length);
uint8_t* buffer = readBuffer_.getWritePtr(length);
read = transport_->read(buffer, length);
UINT32_MAX is the only bound, so the fourteen bytes of a frame header carrying
a 64-bit length decide the size of the allocation. Measured on a Linux x86-64
build: a 14-byte header declaring 0xFFFFFFFF takes the process from a VmPeak of
10,512 kB to 4,205,092 kB and asks the transport underneath for 4,294,967,295
bytes, with no payload sent at all. resetBuffer() constructs a fresh
TMemoryBuffer of that size, so the allocation happens whether or not the bytes
ever arrive, and the buffer is a member that lives as long as the connection.
The transport has a TConfiguration and consults neither of its limits on this
path.
The fix holds the declared length to TConfiguration::getMaxFrameSize() and
refuses anything above it with close code 1009 Message Too Big, which is
already what this transport sends when a frame is too large for it (the
UINT32_MAX arm). That is the same ceiling TFramedTransport::readFrame applies
to its own frames in lib/cpp/src/thrift/transport/TBufferTransports.cpp.
Compatibility, worth a release note: maxFrameSize defaults to 16384000 bytes,
so a peer sending a single WebSocket frame larger than that is now refused
where it was accepted before. Operators who need larger frames raise
TConfiguration::maxFrameSize; the bound follows whatever they set.
Tests are in lib/cpp/test/TWebSocketServerTest.cpp, the first tests this
transport has had. They assert the largest read the server asked of the
transport underneath it rather than merely that the read failed: a payload that
never arrives ends the frame either way, so "did it fail?" passes on the
unmodified library too. Two of the six cases fail before the change (67108864 >
1024 and 32768 > 1024) and all six pass after; the other four are regression
guards that pass either way.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
