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.

Author: Flarna

lib/_http_common.js

@@ -189,6 +189,7 @@ function freeParser(parser, req, socket) {
   if (parser) {
     if (parser._consumed)
       parser.unconsume();
+    parser.markFreed();
     cleanParser(parser);
     parser.remove();
     if (parsers.free(parser) === false) {

src/node_http_parser.cc

@@ -623,6 +623,12 @@ class Parser : public AsyncWrap, public StreamListener {
     parser->EmitDestroy();
   }
 
+  static void MarkFreed(const FunctionCallbackInfo<Value>& args) {
+    Parser* parser;
+    ASSIGN_OR_RETURN_UNWRAP(&parser, args.This());
+    parser->is_being_freed_ = true;
+  }
+
   // TODO(@anonrig): Add V8 Fast API
   static void Remove(const FunctionCallbackInfo<Value>& args) {
     Parser* parser;
@@ -1012,6 +1018,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 +1063,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 +1083,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();
@@ -1332,6 +1343,7 @@ void CreatePerIsolateProperties(IsolateData* isolate_data,
   t->Inherit(AsyncWrap::GetConstructorTemplate(isolate_data));
   SetProtoMethod(isolate, t, "close", Parser::Close);
   SetProtoMethod(isolate, t, "free", Parser::Free);
+  SetProtoMethod(isolate, t, "markFreed", Parser::MarkFreed);
   SetProtoMethod(isolate, t, "remove", Parser::Remove);
   SetProtoMethod(isolate, t, "execute", Parser::Execute);
   SetProtoMethod(isolate, t, "finish", Parser::Finish);
@@ -1401,6 +1413,7 @@ void RegisterExternalReferences(ExternalReferenceRegistry* registry) {
   registry->Register(Parser::New);
   registry->Register(Parser::Close);
   registry->Register(Parser::Free);
+  registry->Register(Parser::MarkFreed);
   registry->Register(Parser::Remove);
   registry->Register(Parser::Execute);
   registry->Register(Parser::Finish);

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