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