Repository: nodejs/node
Pull request: 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.

Fix/test was created using AI support.

Refs: https://github.com/nodejs/node/pull/61995#issuecomment-3975819280


Changed files:
filename: lib/_http_common.js
patch: @@ -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().

filename: src/node_http_parser.cc
patch: @@ -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();

filename: test/parallel/test-http-parser-freed-during-execute.js
patch: @@ -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',
+  );
+}));