[
https://issues.apache.org/jira/browse/THRIFT-6177?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Jens Geyer resolved THRIFT-6177.
--------------------------------
Assignee: Jens Geyer
Resolution: Fixed
> 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
> Assignee: Jens Geyer
> Priority: Major
> Fix For: 0.25.0
>
> Time Spent: 0.5h
> Remaining Estimate: 0h
>
> 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)
