mirror of
https://github.com/nodejs/node.git
synced 2026-10-14 10:26:26 -04:00
http: fix use-after-free when freeParser is called during llhttp_execute
When pipelined requests arrive in one TCP segment, llhttp_execute() parses them all in a single call. If a synchronous 'close' event handler invokes freeParser() mid-execution, cleanParser() nulls out parser state while llhttp_execute() is still on the stack, crashing on the next callback. Add an is_being_freed_ flag that freeParser() sets via parser.markFreed() before cleaning state. Proxy::Raw checks the flag before every callback and returns HPE_USER to abort execution early if set. PR-URL: https://github.com/nodejs/node/pull/62095 Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
This commit is contained in:
1 parent
8edeff9aa7
commit
a06e789625
3 files changed
+31
-1
No files matched your search
+1
-1
@@ -189,8 +189,8 @@ function freeParser(parser, req, socket) {
|
||||
if (parser) {
|
||||
if (parser._consumed)
|
||||
parser.unconsume();
|
||||
cleanParser(parser);
|
||||
parser.remove();
|
||||
cleanParser(parser);
|
||||
if (parsers.free(parser) === false) {
|
||||
// Make sure the parser's stack has unwound before deleting the
|
||||
// corresponding C++ object through .close().
|
||||
|
||||
@@ -628,6 +628,8 @@ class Parser : public AsyncWrap, public StreamListener {
|
||||
Parser* parser;
|
||||
ASSIGN_OR_RETURN_UNWRAP(&parser, args.This());
|
||||
|
||||
parser->is_being_freed_ = true;
|
||||
|
||||
if (parser->connectionsList_ != nullptr) {
|
||||
parser->connectionsList_->Pop(parser);
|
||||
parser->connectionsList_->PopActive(parser);
|
||||
@@ -1012,6 +1014,7 @@ class Parser : public AsyncWrap, public StreamListener {
|
||||
num_values_ = 0;
|
||||
have_flushed_ = false;
|
||||
got_exception_ = false;
|
||||
is_being_freed_ = false;
|
||||
headers_completed_ = false;
|
||||
max_http_header_size_ = max_http_header_size;
|
||||
}
|
||||
@@ -1056,6 +1059,7 @@ class Parser : public AsyncWrap, public StreamListener {
|
||||
size_t num_values_;
|
||||
bool have_flushed_;
|
||||
bool got_exception_;
|
||||
bool is_being_freed_ = false;
|
||||
size_t current_buffer_len_;
|
||||
const char* current_buffer_data_;
|
||||
bool headers_completed_ = false;
|
||||
@@ -1075,6 +1079,9 @@ class Parser : public AsyncWrap, public StreamListener {
|
||||
struct Proxy<int (Parser::*)(Args...), Member> {
|
||||
static int Raw(llhttp_t* p, Args ... args) {
|
||||
Parser* parser = ContainerOf(&Parser::parser_, p);
|
||||
if (parser->is_being_freed_) {
|
||||
return 0;
|
||||
}
|
||||
int rv = (parser->*Member)(std::forward<Args>(args)...);
|
||||
if (rv == 0) {
|
||||
rv = parser->MaybePause();
|
||||
|
||||
@@ -0,0 +1,23 @@
|
||||
'use strict';
|
||||
|
||||
const common = require('../common');
|
||||
const { createServer } = require('http');
|
||||
const { connect } = require('net');
|
||||
|
||||
// Regression test: ensure llhttp_execute() is aborted when freeParser() is
|
||||
// called synchronously during parsing of pipelined requests.
|
||||
const server = createServer(common.mustCall((req, res) => {
|
||||
req.socket.emit('close');
|
||||
res.end();
|
||||
}, 1));
|
||||
|
||||
server.unref();
|
||||
|
||||
server.listen(0, common.mustCall(() => {
|
||||
// Two pipelined requests in one write, processed by a single llhttp_execute().
|
||||
const client = connect(server.address().port);
|
||||
client.end(
|
||||
'GET / HTTP/1.1\r\nHost: localhost\r\nConnection: keep-alive\r\n\r\n' +
|
||||
'GET / HTTP/1.1\r\nHost: localhost\r\nConnection: close\r\n\r\n',
|
||||
);
|
||||
}));
|
||||
Reference in new issue
Block a user