From a29a151d0140d095742d21a004023d024fe93259 Mon Sep 17 00:00:00 2001
From: Matteo Collina <hello@matteocollina.com>
Date: Mon, 18 Jul 2022 10:26:47 +0200
Subject: [PATCH] Merge pull request from GHSA-3cvr-822r-rqcc

* Sanitize \r\n in headers

Signed-off-by: Matteo Collina <hello@matteocollina.com>

* fixup, use regexp

Signed-off-by: Matteo Collina <hello@matteocollina.com>

* fixup, handle method and path too

Signed-off-by: Matteo Collina <hello@matteocollina.com>
---
 lib/core/request.js |  29 +++++++++++
 test/client.js      | 116 ++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 145 insertions(+)

diff --git a/lib/core/request.js b/lib/core/request.js
index 0a3d8558958..4dc2fcca0db 100644
--- a/lib/core/request.js
+++ b/lib/core/request.js
@@ -7,6 +7,27 @@ const {
 const assert = require('assert')
 const util = require('./util')
 
+// tokenRegExp and headerCharRegex have been lifted from
+// https://github.com/nodejs/node/blob/main/lib/_http_common.js
+
+/**
+ * Verifies that the given val is a valid HTTP token
+ * per the rules defined in RFC 7230
+ * See https://tools.ietf.org/html/rfc7230#section-3.2.6
+ */
+const tokenRegExp = /^[\^_`a-zA-Z\-0-9!#$%&'*+.|~]+$/
+
+/**
+ * Matches if val contains an invalid field-vchar
+ *  field-value    = *( field-content / obs-fold )
+ *  field-content  = field-vchar [ 1*( SP / HTAB ) field-vchar ]
+ *  field-vchar    = VCHAR / obs-text
+ */
+const headerCharRegex = /[^\t\x20-\x7e\x80-\xff]/
+
+// Verifies that a given path is valid does not contain control chars \x00 to \x20
+const invalidPathRegex = /[^\u0021-\u00ff]/
+
 const kHandler = Symbol('handler')
 
 const channels = {}
@@ -54,10 +75,14 @@ class Request {
       method !== 'CONNECT'
     ) {
       throw new InvalidArgumentError('path must be an absolute URL or start with a slash')
+    } else if (invalidPathRegex.exec(path) !== null) {
+      throw new InvalidArgumentError('invalid request path')
     }
 
     if (typeof method !== 'string') {
       throw new InvalidArgumentError('method must be a string')
+    } else if (tokenRegExp.exec(method) === null) {
+      throw new InvalidArgumentError('invalid request method')
     }
 
     if (upgrade && typeof upgrade !== 'string') {
@@ -301,6 +326,10 @@ function processHeader (request, key, val) {
     key.toLowerCase() === 'expect'
   ) {
     throw new NotSupportedError('expect header not supported')
+  } else if (tokenRegExp.exec(key) === null) {
+    throw new InvalidArgumentError('invalid header key')
+  } else if (headerCharRegex.exec(val) !== null) {
+    throw new InvalidArgumentError(`invalid ${key} header`)
   } else {
     request.headers += `${key}: ${val}\r\n`
   }
diff --git a/test/client.js b/test/client.js
index a678b629fad..645ff50450f 100644
--- a/test/client.js
+++ b/test/client.js
@@ -1994,3 +1994,119 @@ function buildParams (path) {
 
   return builtParams
 }
+
+test('\\r\\n in Headers', (t) => {
+  t.plan(1)
+
+  const reqHeaders = {
+    bar: '\r\nbar'
+  }
+
+  const client = new Client('http://localhost:4242', {
+    keepAliveTimeout: 300e3
+  })
+  t.teardown(client.close.bind(client))
+
+  client.request({
+    path: '/',
+    method: 'GET',
+    headers: reqHeaders
+  }, (err) => {
+    t.equal(err.message, 'invalid bar header')
+  })
+})
+
+test('\\r in Headers', (t) => {
+  t.plan(1)
+
+  const reqHeaders = {
+    bar: '\rbar'
+  }
+
+  const client = new Client('http://localhost:4242', {
+    keepAliveTimeout: 300e3
+  })
+  t.teardown(client.close.bind(client))
+
+  client.request({
+    path: '/',
+    method: 'GET',
+    headers: reqHeaders
+  }, (err) => {
+    t.equal(err.message, 'invalid bar header')
+  })
+})
+
+test('\\n in Headers', (t) => {
+  t.plan(1)
+
+  const reqHeaders = {
+    bar: '\nbar'
+  }
+
+  const client = new Client('http://localhost:4242', {
+    keepAliveTimeout: 300e3
+  })
+  t.teardown(client.close.bind(client))
+
+  client.request({
+    path: '/',
+    method: 'GET',
+    headers: reqHeaders
+  }, (err) => {
+    t.equal(err.message, 'invalid bar header')
+  })
+})
+
+test('\\n in Headers', (t) => {
+  t.plan(1)
+
+  const reqHeaders = {
+    '\nbar': 'foo'
+  }
+
+  const client = new Client('http://localhost:4242', {
+    keepAliveTimeout: 300e3
+  })
+  t.teardown(client.close.bind(client))
+
+  client.request({
+    path: '/',
+    method: 'GET',
+    headers: reqHeaders
+  }, (err) => {
+    t.equal(err.message, 'invalid header key')
+  })
+})
+
+test('\\n in Path', (t) => {
+  t.plan(1)
+
+  const client = new Client('http://localhost:4242', {
+    keepAliveTimeout: 300e3
+  })
+  t.teardown(client.close.bind(client))
+
+  client.request({
+    path: '/\n',
+    method: 'GET'
+  }, (err) => {
+    t.equal(err.message, 'invalid request path')
+  })
+})
+
+test('\\n in Method', (t) => {
+  t.plan(1)
+
+  const client = new Client('http://localhost:4242', {
+    keepAliveTimeout: 300e3
+  })
+  t.teardown(client.close.bind(client))
+
+  client.request({
+    path: '/',
+    method: 'GET\n'
+  }, (err) => {
+    t.equal(err.message, 'invalid request method')
+  })
+})