http: stricter Transfer-Encoding and header separator parsing

Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Vladimir de Turckheim <vlad2t@hotmail.com>
PR-URL: #315
Backport-PR-URL: #327
CVE-ID: CVE-2022-32215,CVE-2022-32214,CVE-2022-32213

Author: ShogunPanda

deps/llhttp/include/llhttp.h

@@ -3,7 +3,7 @@
 
 #define LLHTTP_VERSION_MAJOR 2
 #define LLHTTP_VERSION_MINOR 1
-#define LLHTTP_VERSION_PATCH 4
+#define LLHTTP_VERSION_PATCH 5
 
 #ifndef LLHTTP_STRICT_MODE
 # define LLHTTP_STRICT_MODE 0
@@ -58,6 +58,7 @@ enum llhttp_errno {
   HPE_OK = 0,
   HPE_INTERNAL = 1,
   HPE_STRICT = 2,
+  HPE_CR_EXPECTED = 25,
   HPE_LF_EXPECTED = 3,
   HPE_UNEXPECTED_CONTENT_LENGTH = 4,
   HPE_CLOSED_CONNECTION = 5,
@@ -78,7 +79,7 @@ enum llhttp_errno {
   HPE_CB_CHUNK_COMPLETE = 20,
   HPE_PAUSED = 21,
   HPE_PAUSED_UPGRADE = 22,
-  HPE_USER = 23
+  HPE_USER = 24
 };
 typedef enum llhttp_errno llhttp_errno_t;
 
@@ -153,6 +154,7 @@ typedef enum llhttp_method llhttp_method_t;
   XX(0, OK, OK) \
   XX(1, INTERNAL, INTERNAL) \
   XX(2, STRICT, STRICT) \
+  XX(25, CR_EXPECTED, CR_EXPECTED) \
   XX(3, LF_EXPECTED, LF_EXPECTED) \
   XX(4, UNEXPECTED_CONTENT_LENGTH, UNEXPECTED_CONTENT_LENGTH) \
   XX(5, CLOSED_CONNECTION, CLOSED_CONNECTION) \
@@ -173,7 +175,7 @@ typedef enum llhttp_method llhttp_method_t;
   XX(20, CB_CHUNK_COMPLETE, CB_CHUNK_COMPLETE) \
   XX(21, PAUSED, PAUSED) \
   XX(22, PAUSED_UPGRADE, PAUSED_UPGRADE) \
-  XX(23, USER, USER) \
+  XX(24, USER, USER) \
 
 
 #define HTTP_METHOD_MAP(XX) \

deps/llhttp/src/llhttp.c

@@ -13,7 +13,7 @@ Content-Type: text/plain; charset=utf-8
 Host: hacker.exploit.com
 Connection: keep-alive
 Content-Length: 10
-Transfer-Encoding: chunked, eee
+Transfer-Encoding: eee, chunked
 
 HELLOWORLDPOST / HTTP/1.1
 Content-Type: text/plain; charset=utf-8

test/parallel/test-http-invalid-te.js

@@ -0,0 +1,43 @@
+'use strict';
+
+const common = require('../common');
+const assert = require('assert');
+
+const http = require('http');
+const net = require('net');
+
+const msg = [
+  'GET / HTTP/1.1',
+  'Host: localhost',
+  'Dummy: x\nContent-Length: 23',
+  '',
+  'GET / HTTP/1.1',
+  'Dummy: GET /admin HTTP/1.1',
+  'Host: localhost',
+  '',
+  '',
+].join('\r\n');
+
+const server = http.createServer(common.mustNotCall());
+
+server.listen(0, common.mustSucceed(() => {
+  const client = net.connect(server.address().port, 'localhost');
+
+  let response = '';
+
+  client.on('data', common.mustCall((chunk) => {
+    response += chunk.toString('utf-8');
+  }));
+
+  client.setEncoding('utf8');
+  client.on('error', common.mustNotCall());
+  client.on('end', common.mustCall(() => {
+    assert.strictEqual(
+      response,
+      'HTTP/1.1 400 Bad Request\r\nConnection: close\r\n\r\n'
+    );
+    server.close();
+  }));
+  client.write(msg);
+  client.resume();
+}));

test/parallel/test-http-missing-header-separator-cr.js

@@ -0,0 +1,51 @@
+'use strict';
+
+const common = require('../common');
+const assert = require('assert');
+
+const http = require('http');
+const net = require('net');
+
+const msg = [
+  'POST / HTTP/1.1',
+  'Host: 127.0.0.1',
+  'Transfer-Encoding: chunkedchunked',
+  '',
+  '1',
+  'A',
+  '0',
+  '',
+].join('\r\n');
+
+const server = http.createServer(common.mustCall((req, res) => {
+  // Verify that no data is received
+
+  req.on('data', common.mustNotCall());
+
+  req.on('end', common.mustNotCall(() => {
+    res.writeHead(200, { 'Content-Type': 'text/plain' });
+    res.end();
+  }));
+}, 1));
+
+server.listen(0, common.mustSucceed(() => {
+  const client = net.connect(server.address().port, 'localhost');
+
+  let response = '';
+
+  client.on('data', common.mustCall((chunk) => {
+    response += chunk.toString('utf-8');
+  }));
+
+  client.setEncoding('utf8');
+  client.on('error', common.mustNotCall());
+  client.on('end', common.mustCall(() => {
+    assert.strictEqual(
+      response,
+      'HTTP/1.1 400 Bad Request\r\nConnection: close\r\n\r\n'
+    );
+    server.close();
+  }));
+  client.write(msg);
+  client.resume();
+}));

test/parallel/test-http-transfer-encoding-repeated-chunked.js

@@ -1,6 +1,7 @@
 'use strict';
 
 const common = require('../common');
+const assert = require('assert');
 
 const http = require('http');
 const net = require('net');
@@ -22,23 +23,30 @@ const msg = [
   '',
 ].join('\r\n');
 
-// Verify that the server is called only once even with a smuggled request.
-
-const server = http.createServer(common.mustCall((req, res) => {
+const server = http.createServer(common.mustNotCall((req, res) => {
   res.end();
 }, 1));
 
-function send(next) {
+server.listen(0, common.mustSucceed(() => {
   const client = net.connect(server.address().port, 'localhost');
+
+  let response = '';
+
+  // Verify that the server listener is never called
+
+  client.on('data', common.mustCall((chunk) => {
+    response += chunk.toString('utf-8');
+  }));
+
   client.setEncoding('utf8');
   client.on('error', common.mustNotCall());
-  client.on('end', next);
-  client.write(msg);
-  client.resume();
-}
-
-server.listen(0, common.mustSucceed(() => {
-  send(common.mustCall(() => {
+  client.on('end', common.mustCall(() => {
+    assert.strictEqual(
+      response,
+      'HTTP/1.1 400 Bad Request\r\nConnection: close\r\n\r\n'
+    );
     server.close();
   }));
+  client.write(msg);
+  client.resume();
 }));