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: #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>

Author: Flarna

lib/_http_common.js

@@ -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().

src/node_http_parser.cc

@@ -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);
@@ -1011,6 +1013,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;
   }
@@ -1055,6 +1058,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;
@@ -1074,6 +1078,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();

test/parallel/test-http-parser-freed-during-execute.js

@@ -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',
+  );
+}));