(Collaborative post by hebi and Gynvael Coldwind)
Crow is an asynchronous C++ HTTP/WebSocket framework for creating "flask-like" web services. In early August we discovered a pretty interesting use-after-free vulnerability. Since Crow takes advantage of the Asio library for asynchronous input/output operations, analysis of this vulnerability took a few long evenings since the cause was split between multiple interweaved tasks and callbacks. Eventually we traced the root cause to an interesting mismatch between two layers of code, one of which - the HTTP parser - was supporting HTTP pipelining (or rather was agnostic towards it, which resulted in pipelining being inadvertently supported), while the other - HTTP server logic - was not designed to take HTTP pipelining into account. This resulted in some interesting "race conditions" with one task "thinking" an HTTP connection was over (and deleting objects) while another still using them while processing a separate HTTP request.
One thing to note is that we never proved exploitability (as in: actual RCE, since it's pretty easy to just trigger this vulnerability to cause a DoS) due to ENOTIME, though we believe it should be possible, if highly complex.
The vulnerability in question was reported mid-August and fixed within 6 days.
CVSS, CVE, etc
Human readable details are in the next section.
- CVE: CVE-2022-38667
- CVSS 3.1: 8.1 High (AV:N/AC:H/PR:N/UI:N/S:U/C:H/I:H/A:H) [as originally reported]
- CVSS 3.1: 9.8 Critical (AV:N/AC:L/PR:N/UI:N/S:U/C:H/I:H/A:H) [as rated by NIST/NVD]
Timeline
- 2022-08-11: Vulnerability discovered.
- 2022-08-17: Vulnerability reported.
- 2022-08-21: Public fix was proposed.
- 2022-08-22: Public fix was merged in.
- 2022-08-22: CVE requested and assigned.
- 2022-09-23: Details were published.
Original report with details
*** Summary:
Affected: Crow version 1.0+4 and older
https://github.com/CrowCpp/Crow - maintained version (fork)
https://github.com/ipkn/crow - original version
Discovered by:
hebi
Gynvael Coldwind
Method of discovery:
Fuzzing, Manual Analysis (reading the code)
"Crow is a C++ framework for creating HTTP or Websocket web services. It uses
routing similar to Python's Flask which makes it easy to use. It is also
extremely fast, beating multiple existing C++ frameworks as well as non C++
frameworks." (source: project's README.md)
Crow version 1.0+4 and older are vulnerable to a use-after-free condition due to
incorrect Connection object lifetime management (though partial HTTP pipelining
support is the deeper underlying problem). On successful exploitation this can
potentially lead to either Remote Code Execution or Information Disclosure (a
failed exploitation will lead to a Denial of Service as the server will crash).
See sections below for details.
IMPORTANT: This vulnerability is reported under the 90-day policy
(version 2021), i.e. this report will be shared publicly with the defensive
community on 16th November 2022 if a patch/fix is not available by that time, or
30 days after the fix becomes available. For details please see:
https://googleprojectzero.blogspot.com/2021/04/policy-and-disclosure-2021-edition.html
*** Crow execution model and life of a request
Note: If you understand Crow's execution model you can safely skip this section.
Note: The description below is a bit simplified and contains only the
information relevant to the vulnerability.
Crow is using an asynchronous queued task execution model (implemented using the
Asio C++ library - https://think-async.com/Asio/), which is not unlike typical
JavaScript model.
Technically there are actually N+1 task queues (where N denotes the number of
threads chosen by the application developer) - one per each worker thread and
an extra "main" queue for some timers, signals, etc.
When a new client is connected, the connection (represented by a Connection
object) is bound to a certain thread and its task queue. This means that all
processing done by that connection is going to happen within the constrains of
the same thread.
In case the server handles more simultaneous connections than there are threads,
each thread will handle tasks related to multiple connections. On the flip side,
if there are very few connections, then basically a connection has a task queue
(and thread) for itself.
After the connection is established (and a Connection object made) a task
tasked with receiving some data is added to the task queue. Once it completes,
the completion callback is called (that's the lambda function inside the
Connection::do_read() method in http_connection.h).
When data is received this callback passes the data to the parser (call to
parser_.feed() method; the implementation is split between parser.h and
http_parser_merged.h).
The parser (still operating within the same task) can then invoke a set of
callbacks on various parsing events like method received, URL received,
header name / value received, and so on. While all of these callbacks
are technically processed within parser.h, some of them invoke methods of the
Connection object.
Sooner (error) or later (no error) one of these called methods will end up with
a prepared ready-to-send response (e.g. a 404 response on error or 200 response
with some app-generated data on success). In such case the response will either
be immediately (synchronously) sent out or a new task will be added to the task
queue to send the response out later and call a completion callback (see the
lambda function inside the Connection::do_write()'s method).
Once the parser returns, it might either require more data (in which case a new
task to receive more data will be added to the task queue) or decide it's time
to close the connection and finish the task.
By the way...
On 22nd Nov'24 we're running a webinar called "CVEs of SSH" – it's free, but requires sign up: https://hexarcana.ch/workshops/cves-of-ssh (Dan from HexArcana is the speaker).
Next, if any data was asynchronously being scheduled to be sent out, a
completion callback (do_write()'s lambda) might be called. The completion
callback clears the response buffers and might close the connection as well if
needed.
The last task related to the given connection will delete the Connection object.
To summarize, the list of tasks executed while processing a connection might
look like this:
async_accept →
async_accept's completion cb (creates Connection object) →
async_receive (receive HTTP request) →
async_receive's completion cb (invokes parser/schedules sending) →
async_write (send HTTP response) →
aync_write's completion cb (deletes Connection object if needed).
There might be more async_receive related tasks if data is trickling in slowly
(multiple receives per HTTP request), and multiple async_write tasks if there
are multiple responses to be sent out (Connection: keep-alive).
As mentioned, the last task is responsible for deleting the Connection object.
To control and decide when that should happen a set of two flags is used:
Connection::is_reading
Connection::is_writing
If any receiving task is scheduled, is_reading flag is set.
If any sending task is scheduled, is_writing flag is set.
Once the tasks are complete and the connection is being closed, respective flags
are cleared.
In various places a method called Connection::check_destroy() is called, which
checks the status of both these flags. If neither is set (i.e. both are
cleared), then the Connection object is deemed to be no longed needed and is
deleted.
*** High-level root cause
In essence, the vulnerability in this report boils down to incomplete HTTP
pipelining support.
When using HTTP pipelining the client sends multiple HTTP requests as part of
the same connection one after another without waiting to receive HTTP responses
for any of the previous requests (in opposition to the "normal" way of doing it,
where only one request is sent, then the client waits for the complete response,
and only then it proceeds with the next request). In theory this allows HTTP
servers to process requests in parallel and then just send replies in order.
Historically though HTTP pipelining proved to be tricky to implement correctly
on both the client and server side. As such none of popular modern browsers have
it enabled by default.
In case of Crow the HTTP parser does support HTTP pipelining, or rather it
continues to invoke callbacks for all HTTP packets that are present in the
received TCP packet (perhaps it's more fair to say it's HTTP pipelining
agnostic).
That being said, the asynchronous Connection layer is unaware of pipelining,
which leads to various logic errors born from more than one HTTP request being
handled at the same time, while being limited to a state tailored to a single
request and response.
The simplest (and benign) way to demonstrate this is to create two endpoints
that return 512KB of "1" and "2", as shown below:
CROW_ROUTE(app, "/one")
([] {
return std::string(1024 * 512, '1');
});
CROW_ROUTE(app, "/two")
([] {
return std::string(1024 * 512, '2');
});
And then request data from both endpoints in the same TCP packet:
GET /one HTTP/1.1
Host: example.com
GET /two HTTP/1.1
Host: example.com
In the response we would expect (in this order):
1. HTTP response header for endpoint /one.
2. 512KB of "1"s.
3. HTTP response header for endpoint /two.
4. 512KB of "2"s.
Here's what we actually receive:
$ cat packet.http - | nc -v 127.0.0.1 18080 | hexdump -C
Connection to 127.0.0.1 18080 port [tcp/*] succeeded!
00000000 48 54 54 50 2f 31 2e 31 20 32 30 30 20 4f 4b 0d |HTTP/1.1 200 OK.|
00000010 0a 43 6f 6e 74 65 6e 74 2d 4c 65 6e 67 74 68 3a |.Content-Length:|
00000020 20 35 32 34 32 38 38 0d 0a 53 65 72 76 65 72 3a | 524288..Server:|
00000030 20 43 72 6f 77 2f 6d 61 73 74 65 72 0d 0a 44 61 | Crow/master..Da|
00000040 74 65 3a 20 54 75 65 2c 20 31 36 20 41 75 67 20 |te: Tue, 16 Aug |
00000050 32 30 32 32 20 31 39 3a 33 36 3a 34 38 20 47 4d |2022 19:36:48 GM|
00000060 54 0d 0a 43 6f 6e 6e 65 63 74 69 6f 6e 3a 20 4b |T..Connection: K|
00000070 65 65 70 2d 41 6c 69 76 65 0d 0a 0d 0a 31 31 31 |eep-Alive....111|
00000080 31 31 31 31 31 31 31 31 31 31 31 31 31 31 31 31 |1111111111111111|
*
00010000 48 54 54 50 2f 31 2e 31 20 32 30 30 20 4f 4b 0d |HTTP/1.1 200 OK.|
00010010 0a 63 6f 6e 6e 65 63 74 69 6f 6e 3a 20 4b 65 65 |.connection: Kee|
00010020 70 2d 41 6c 69 76 65 0d 0a 43 6f 6e 74 65 6e 74 |p-Alive..Content|
00010030 2d 4c 65 6e 67 74 68 3a 20 30 0d 0a 53 65 72 76 |-Length: 0..Serv|
00010040 65 72 3a 20 43 72 6f 77 2f 6d 61 73 74 65 72 0d |er: Crow/master.|
00010050 0a 44 61 74 65 3a 20 54 75 65 2c 20 31 36 20 41 |.Date: Tue, 16 A|
00010060 75 67 20 32 30 32 32 20 31 39 3a 33 36 3a 34 38 |ug 2022 19:36:48|
00010070 20 47 4d 54 0d 0a 43 6f 6e 6e 65 63 74 69 6f 6e | GMT..Connection|
00010080 3a 20 4b 65 65 70 2d 41 6c 69 76 65 0d 0a 31 31 |: Keep-Alive..11|
00010090 31 31 31 31 31 31 31 31 31 31 31 31 31 31 31 31 |1111111111111111|
*
00020080 31 31 31 31 31 31 31 31 31 31 31 31 31 31 0d 0a |11111111111111..|
00020090 31 31 31 31 31 31 31 31 31 31 31 31 31 31 31 31 |1111111111111111|
*
00080100 31 31 31 31 31 31 31 31 31 31 31 31 31 |1111111111111|
0008010d
Note: The * sign above denotes a repeated row.
To present the response in a form of the list, we get:
1. HTTP response header for endpoint /one.
2. 64KB (minus response header size) of "1"s.
3. HTTP response header for endpoint /two with Content-Length set to 0.
4. The remaining amount of "1"s.
Pretty obviously responses to both packets were mixed, both in sending (HTTP
response header of /two injected in the middle of /one's response) and in
processing (Content-Length: 0 in /two's response).
Note: As mentioned before, thankfully modern browsers disabled HTTP pipelining.
Otherwise the above behavior under certain circumstances would allow for
a UXSS (or straight-up JS injection).
To summarize, the vulnerability discussed below exploits the fact that the
Connection layer isn't aware it processes multiple requests before ever
finalizing previous ones.
*** Vulnerability (use-after-free)
As mentioned before, if both the is_reading and is_writing flags are cleared,
then the next call to Connection::check_destroy() will delete the Connection
object. In case there would still be another task related to the Connection
scheduled to be executed, this would lead to a use-after-free vulnerability (and
potentially also a double-free in case Connection::check_destroy() is invoked
again).
One of the discovered ways to achieve this is to send two HTTP requests in one
TCP packet:
1. The first requests any dynamic endpoint whose response is smaller than
1MB (required to fall into a specific code path in
Connection::do_write_general()).
2. The second is both an invalid HTTP request (e.g. missing Host header) and
is referring to a non-existing endpoint (to get a 404 error response).
For example:
GET /endpoint HTTP/1.1
Host: 127.0.0.1
GET /idontexist HTTP/1.1
The execution for the above example will flow as follows:
async_accept →
async_accept's completion cb →
async_receive (receive HTTP request) →
- The is_reading flag is set.
async_receive's completion cb (lambda in do_read())
- The method parser_::feed() is invoked with the received data.
- ...
- Parser calls on_url callback, which in turn calls
Connection::handle_url(), which establishes a route for /endpoint.
- ...
- Parser calls on_message_complete callback, which in turn invokes the
endpoint's handler.
- After the endpoint's handler finishes, the data is scheduled to be
sent (i.e. a new "write" task is added to the task queue). This also
causes the is_writing flag to be set.
- ... parser continues with the second packet ...
- Parser calls on_url callback, which in turn calls
Connection::handle_url(), which establishes that there is no route
specified for /idontexist.
- This in turn causes a 404 packet to be scheduled to be sent to the
client (i.e. a second "write" task is added to the task queue). This
also causes the is_writing to be set (yes, it was already set).
- The Parser notices there are no headers, which is invalid for HTTP/1.1
as at least Host: is required, and exists with an error.
- Given Parser's error, the is_reading is cleared, the socket is closed,
and this tasks finishes (note: Connection object is still alive, since
is_writing is still set). →
async_write (try to send first HTTP response) →
aync_write's completion cb (lambda in do_write())
- The is_writing flag is cleared.
- Since the socket was closed (and therefore sending failed),
Connection::check_destroy() is called.
- Since both is_reading and is_writing flags are cleared, the
Connection object is deleted.
- The task finishes. →
async_write (try to send 404 HTTP response) →
async_write's completion cb (lambda in do_write())
- Attempts to access the deleted Connection object, which by this
point is already freed. Application crashes.
In case of the example packet Crow will likely crash at that point, though it
can potentially already crash when trying to execute the second scheduled
async_write (since the buffers with the response data are already deleted).
While we haven't attempted to exploit this vulnerability beyond triggering it,
we believe successful use-after-free exploitation cannot be ruled out at this
time. Exploitation would likely involve either using two Connections in the
same thread (i.e. using the same task queue) with carefully chosen order of
data transmission. Likely winning a race condition might be requires as well.
Successful exploitation of this use-after-free vulnerability would lead to
Remote Code Execution, i.e. executing arbitrary code chosen by the attacker in
the context of the Crow app's process.
*** Proposed fix:
Given that HTTP pipelining support is not needed (browsers have it blocked,
there is HTTP/2 which supports multiplexing), the best solution is to block
HTTP pipelining altogether.
This would likely require adding a mechanism to make sure that any evidence of
multiple HTTP requests within a single parser_.feed() call (as indicated by
the on_... callbacks being invoked) will result in the connection being dropped
after the first reply, and no extra HTTP request will be processed.
*** Other worrying patterns
As mentioned before, the Connection::check_destroy() function potentially
deletes the Connection object. Given this, if the object is deleted, no other
operations can be performed on it (i.e. any other operation is technically a
use-after-free).
That said, there are 3 additional places where check_destroy() is called, but
operations are still made on the object regardless.
It must be said that we were not able to find a way to actually trigger these
code paths, but these do look like accidents bound to happen some day, so
perhaps some refactoring in the area might be in order.
Place 1: End of do_write_static() function
if (close_connection_)
{
adaptor_.shutdown_readwrite();
adaptor_.close();
CROW_LOG_DEBUG << this << " from write (static)";
check_destroy();
}
res.end(); // Connection is potentially already deleted here.
res.clear();
buffers_.clear();
}
Place 2: End of do_write_general() for large (over 1MB) responses
if (close_connection_)
{
adaptor_.shutdown_readwrite();
adaptor_.close();
CROW_LOG_DEBUG << this << " from write (res_stream)";
check_destroy();
}
res.end(); // Connection is potentially already deleted here.
res.clear();
buffers_.clear();
}
}
Place 3: do_write_sync()'s completion lambda (executed before function returns)
Note: In this case the issue lies in functions calling do_write_sync() not being
aware that the Connection object might be potentially deleted in this
method.
else
{
CROW_LOG_ERROR << ec << " - happened while sending buffers";
CROW_LOG_DEBUG << this << " from write (sync)(2)";
check_destroy();
return true;
}
});
Perhaps making check_destroy() return a boolean value indicating whether the
object was or wasn't deleted might be a solution. That way the calling functions
would know whether to immediately exit, or continue execution.
Add a comment: