[jira] [Created] (THRIFT-6179) C++ WebSocket server recurses once per Ping frame with no depth bound

["Jens Geyer (Jira)" <ji...@apache.org>]

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)