Jens Geyer created THRIFT-6179:
----------------------------------
Summary: C++ WebSocket server recurses once per Ping frame with no
depth bound
Key: THRIFT-6179
URL: https://issues.apache.org/jira/browse/THRIFT-6179
Project: Thrift
Issue Type: Bug
Components: C++ - Library
Reporter: Jens Geyer
Fix For: 0.25.0
TWebSocketServer::readFrame answers a Ping by calling itself
(lib/cpp/src/thrift/transport/TWebSocketServer.h):
case Opcode::Ping:
pong();
return readFrame();
A Ping carries no data for the caller, so the reader has to move on to the next
frame to satisfy it. Doing that by re-entering readFrame costs one stack frame
per Ping, and nothing bounds how many a peer may send.
Measured against master, with a client sending Ping frames of one payload byte
-- 7 bytes each on the wire -- followed by one ordinary data frame:
20,000 pings (140 kB) -> 3,200,344 bytes of stack
40,000 pings (280 kB) -> 6,400,344 bytes of stack
60,000 pings (420 kB) -> segmentation fault
That is 160 bytes of stack per 7 bytes of input, and an 8 MB stack -- the Linux
default -- is exhausted somewhere around 52,000 pings, roughly 366 kB. It is
not build-dependent: gcc 11 does not eliminate the call at -O2 any more than at
-O0, and both builds fault at the same point.
A loop reads every frame from the same depth, which is the same shape the Go
framed and header transports were given for their own re-entry (THRIFT-6157,
THRIFT-6158). No new configuration is involved and no legitimate traffic
changes: pings are still answered, one after another, for as long as a peer
cares to send them.
Note for whoever picks this up: a Ping with no payload does not currently reach
this path at all, because readFrame does not consume the four masking-key bytes
of a zero-length masked frame and the stream desynchronises on the next header
instead. That is a separate defect, filed separately; the measurements above
use one-byte pings so that they exercise this one.
Test in lib/cpp/test/TWebSocketServerTest.cpp: a run of pings followed by a
data frame is read with the data frame intact, and the transport underneath
reports the depth the reader reached.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
