fix: guard idle socket validation to skip fresh sockets

mcollina · UlisesGascon · mcollina · commit ea8930cf3745 · 2026-06-08T14:44:30.000+02:00
References: GHSA-35p6-xmwp-9g52 Signed-off-by: Matteo Collina <hello@matteocollina.com> Co-authored-by: Ulises Gascon <ulisesgascongonzalez@gmail.com> Signed-off-by: Matteo Collina <hello@matteocollina.com>
diff --git a/lib/dispatcher/client-h1.js b/lib/dispatcher/client-h1.js
@@ -57,6 +57,9 @@ const constants = require('../llhttp/constants.js')
 const EMPTY_BUF = Buffer.alloc(0)
 const FastBuffer = Buffer[Symbol.species]
 const removeAllListeners = util.removeAllListeners
+const kIdleSocketValidation = Symbol('kIdleSocketValidation')
+const kIdleSocketValidationTimeout = Symbol('kIdleSocketValidationTimeout')
+const kSocketUsed = Symbol('kSocketUsed')
 
 let extractBody
 
@@ -440,6 +443,11 @@ class Parser {
       return -1
     }
 
+    if (client[kRunning] === 0) {
+      util.destroy(socket, new SocketError('bad response', util.getSocketInfo(socket)))
+      return -1
+    }
+
     const request = client[kQueue][client[kRunningIdx]]
     if (!request) {
       return -1
@@ -568,6 +576,11 @@ class Parser {
       return -1
     }
 
+    if (client[kRunning] === 0) {
+      util.destroy(socket, new SocketError('bad response', util.getSocketInfo(socket)))
+      return -1
+    }
+
     const request = client[kQueue][client[kRunningIdx]]
 
     if (!request) {
@@ -746,6 +759,7 @@ class Parser {
     request.onComplete(headers)
 
     client[kQueue][client[kRunningIdx]++] = null
+    socket[kSocketUsed] = client[kPending] === 0
 
     if (socket[kWriting]) {
       assert(client[kRunning] === 0)
@@ -822,6 +836,9 @@ function connectH1 (client, socket) {
   socket[kWriting] = false
   socket[kReset] = false
   socket[kBlocking] = false
+  socket[kIdleSocketValidation] = 0
+  socket[kIdleSocketValidationTimeout] = null
+  socket[kSocketUsed] = false
   socket[kParser] = new Parser(client, socket, llhttpInstance)
 
   util.addListener(socket, 'error', onHttpSocketError)
@@ -864,7 +881,7 @@ function connectH1 (client, socket) {
      * @returns {boolean}
      */
     busy (request) {
-      if (socket[kWriting] || socket[kReset] || socket[kBlocking]) {
+      if (socket[kWriting] || socket[kReset] || socket[kBlocking] || socket[kIdleSocketValidation] === 1) {
         return true
       }
 
@@ -944,6 +961,8 @@ function onHttpSocketEnd () {
 function onHttpSocketClose () {
   const parser = this[kParser]
 
+  clearIdleSocketValidation(this)
+
   if (parser) {
     if (!this[kError] && parser.statusCode && !parser.shouldKeepAlive) {
       this[kError] = parser.finish() || this[kError]
@@ -990,6 +1009,28 @@ function onSocketClose () {
   this[kClosed] = true
 }
 
+function clearIdleSocketValidation (socket) {
+  if (socket[kIdleSocketValidationTimeout]) {
+    clearTimeout(socket[kIdleSocketValidationTimeout])
+    socket[kIdleSocketValidationTimeout] = null
+  }
+
+  socket[kIdleSocketValidation] = 0
+}
+
+function scheduleIdleSocketValidation (client, socket) {
+  socket[kIdleSocketValidation] = 1
+  socket[kIdleSocketValidationTimeout] = setTimeout(() => {
+    socket[kIdleSocketValidationTimeout] = null
+    socket[kIdleSocketValidation] = 2
+
+    if (client[kSocket] === socket && !socket.destroyed) {
+      client[kResume]()
+    }
+  }, 0)
+  socket[kIdleSocketValidationTimeout].unref?.()
+}
+
 /**
  * @param {import('./client.js')} client
  */
@@ -1007,6 +1048,32 @@ function resumeH1 (client) {
       socket[kNoRef] = false
     }
 
+    if (client[kRunning] === 0 && client[kPending] > 0 && socket[kSocketUsed]) {
+      if (socket[kIdleSocketValidation] === 0) {
+        scheduleIdleSocketValidation(client, socket)
+        socket[kParser].readMore()
+        if (socket.destroyed) {
+          return
+        }
+        return
+      }
+
+      if (socket[kIdleSocketValidation] === 1) {
+        socket[kParser].readMore()
+        if (socket.destroyed) {
+          return
+        }
+        return
+      }
+    }
+
+    if (client[kRunning] === 0) {
+      socket[kParser].readMore()
+      if (socket.destroyed) {
+        return
+      }
+    }
+
     if (client[kSize] === 0) {
       if (socket[kParser].timeoutType !== TIMEOUT_KEEP_ALIVE) {
         socket[kParser].setTimeout(client[kKeepAliveTimeoutValue], TIMEOUT_KEEP_ALIVE)
@@ -1105,6 +1172,7 @@ function writeH1 (client, request) {
   }
 
   const socket = client[kSocket]
+  clearIdleSocketValidation(socket)
 
   /**
    * @param {Error} [err]
diff --git a/test/response-queue-poisoning.js b/test/response-queue-poisoning.js
@@ -0,0 +1,52 @@
+'use strict'
+
+const assert = require('node:assert')
+const { createServer } = require('node:http')
+const { after, test } = require('node:test')
+const { Client } = require('..')
+
+function readBody (body) {
+  return new Promise((resolve, reject) => {
+    let data = ''
+    body.setEncoding('latin1')
+    body.on('data', chunk => { data += chunk })
+    body.on('end', () => resolve(data))
+    body.on('error', reject)
+  })
+}
+
+test('should not reuse an idle socket with buffered unsolicited response bytes', async () => {
+  let evilServerSocket
+
+  const server = createServer((req, res) => {
+    if (!evilServerSocket) {
+      evilServerSocket = req.socket
+    }
+
+    res.end(req.url)
+  })
+  after(() => server.close())
+
+  await new Promise(resolve => server.listen(0, resolve))
+
+  const client = new Client(`http://localhost:${server.address().port}`, {
+    keepAliveTimeout: 300e3
+  })
+  after(() => client.close())
+
+  const response1 = await client.request({ path: '/request1', method: 'GET' })
+  assert.strictEqual(await readBody(response1.body), '/request1')
+
+  evilServerSocket.write(
+    'HTTP/1.1 200 OK\r\n' +
+    'Poison-Free-Socket: true\r\n' +
+    'Connection: keep-alive\r\n' +
+    'Keep-Alive: timeout=300\r\n' +
+    'Content-Length: 0\r\n' +
+    '\r\n'
+  )
+
+  const response2 = await client.request({ path: '/request2', method: 'GET' })
+  assert.strictEqual(response2.headers['poison-free-socket'], undefined)
+  assert.strictEqual(await readBody(response2.body), '/request2')
+})
