Fix CVE-2023-28155 by patching request module in nodejs

microsoft · May 25, 2023 · 3e42062 · 3e42062
1 parent ad4a80e
commit 3e42062
Show file tree

Hide file tree

Showing 2 changed files with 217 additions and 2 deletions.
diff --git a/SPECS/nodejs/CVE-2023-28155.patch b/SPECS/nodejs/CVE-2023-28155.patch
@@ -0,0 +1,211 @@
+Fixes CVE-2023-28155: https://nvd.nist.gov/vuln/detail/CVE-2023-28155, which is a vulnerability
+in the the request module that is used by this package.
+
+Note that request is deprecated (see https://github.com/request/request for details), so this
+has not and will never be fixed in the request module itself. However, there is a pull request
+that fixes it.
+
+Adapted by tobiasb@microsoft.com from a pull request for a patch to request:
+    https://github.com/request/request/pull/3444/files
+
+From d42332182512e56ba68446f49c3e3711e04301a2 Mon Sep 17 00:00:00 2001
+From: <redacted>
+Date: Sun, 12 Mar 2023 19:47:24 +0100
+Subject: [PATCH 1/5] Added option "allowInsecureRedirect"
+
+---
+PATCH NOTE -- ORIGINAL:
+ lib/redirect.js          |  6 +++++-
+PATCH NOTE -- UPDATE:
+ deps/npm/node_modules/request/lib/redirect.js |  6 +++++-
+
+PATCH NOTE: These tests are not included in the module we use, so they are not included in this patch.
+ tests/test-httpModule.js |  3 ++-
+ tests/test-redirect.js   | 15 ++++++++++++++-
+
+PATCH NOTE -- ORIGINAL:
+ 3 files changed, 21 insertions(+), 3 deletions(-)
+PATCH NOTE -- UPDATED:
+ 1 file changed, 5 insertions(+), 1 deletion(-)
+
+# PATCH NOTE -- ORIGINAL:
+#diff --git a/lib/redirect.js b/lib/redirect.js
+# PATCH NOTE -- UPDATED with path used within the source tarball:
+diff --git a/deps/npm/node_modules/request/lib/redirect.js b/deps/npm/node_modules/request/lib/redirect.js
+
+index b9150e77c..770c7f41b 100644
+# PATCH NOTE -- ORIGINAL:
+# --- a/lib/redirect.js
+# +++ b/lib/redirect.js
+# PATCH NOTE -- UPDATED with path used within the source tarball:
+--- a/deps/npm/node_modules/request/lib/redirect.js
++++ b/deps/npm/node_modules/request/lib/redirect.js
+
+@@ -14,6 +14,7 @@ function Redirect (request) {
+   this.redirects = []
+   this.redirectsFollowed = 0
+   this.removeRefererHeader = false
++  this.allowInsecureRedirect = false
+ }
+
+ Redirect.prototype.onRequest = function (options) {
+@@ -40,6 +41,9 @@ Redirect.prototype.onRequest = function (options) {
+   if (options.followOriginalHttpMethod !== undefined) {
+     self.followOriginalHttpMethod = options.followOriginalHttpMethod
+   }
++  if (options.allowInsecureRedirect !== undefined) {
++    self.allowInsecureRedirect = options.allowInsecureRedirect;
++  }
+ }
+
+ Redirect.prototype.redirectTo = function (response) {
+@@ -108,7 +112,7 @@ Redirect.prototype.onResponse = function (response) {
+   request.uri = url.parse(redirectTo)
+
+   // handle the case where we change protocol from https to http or vice versa
+-  if (request.uri.protocol !== uriPrev.protocol) {
++  if (request.uri.protocol !== uriPrev.protocol && self.allowInsecureRedirect) {
+     delete request.agent
+   }
+
+# PATCH NOTE: The rest of the diffs are not applied because they are tests and not
+# included in the source tarball.
+# diff --git a/tests/test-httpModule.js b/tests/test-httpModule.js
+# index 4d4e236bf..a59c427b1 100644
+# --- a/tests/test-httpModule.js
+# +++ b/tests/test-httpModule.js
+# @@ -70,7 +70,8 @@ function runTests (name, httpModules) {
+#    tape(name, function (t) {
+#      var toHttps = 'http://localhost:' + plainServer.port + '/to_https'
+#      var toPlain = 'https://localhost:' + httpsServer.port + '/to_plain'
+# -    var options = { httpModules: httpModules, strictSSL: false }
+# +    var options = { httpModules: httpModules, strictSSL: false, allowInsecureRedirect: true }
+# +    var optionsSecure = { httpModules: httpModules, strictSSL: false }
+#      var modulesTest = httpModules || {}
+
+#      clearFauxRequests()
+# diff --git a/tests/test-redirect.js b/tests/test-redirect.js
+# index b7b5ca676..48b4982e4 100644
+# --- a/tests/test-redirect.js
+# +++ b/tests/test-redirect.js
+# @@ -345,7 +345,8 @@ tape('http to https redirect', function (t) {
+#    hits = {}
+#    request.get({
+#      uri: require('url').parse(s.url + '/ssl'),
+# -    rejectUnauthorized: false
+# +    rejectUnauthorized: false,
+# +    allowInsecureRedirect: true
+#    }, function (err, res, body) {
+#      t.equal(err, null)
+#      t.equal(res.statusCode, 200)
+# @@ -354,6 +355,18 @@ tape('http to https redirect', function (t) {
+#    })
+#  })
+
+# +tape('http to https redirect should fail without the explicit "allowInsecureRedirect" option', function (t) {
+# +  hits = {}
+# +  request.get({
+# +    uri: require('url').parse(s.url + '/ssl'),
+# +    rejectUnauthorized: false
+# +  }, function (err, res, body) {
+# +    t.notEqual(err, null)
+# +    t.equal(err.code, "ERR_INVALID_PROTOCOL","Failed to cross-protocol redirect")
+# +    t.end()
+# +  })
+# +})
+# +
+#  tape('should have referer header by default when following redirect', function (t) {
+#    request.post({
+#      uri: s.url + '/temp',
+
+# From 9d69d750f39cc5ab6f3b011e17472bc28b14dc22 Mon Sep 17 00:00:00 2001
+# From: Szymon Drosdzol <szymon@doyensec.com>
+# Date: Sun, 12 Mar 2023 19:50:09 +0100
+# Subject: [PATCH 2/5] Documented allowInsecureRedirect in Readme
+
+# ---
+#  README.md | 1 +
+#  1 file changed, 1 insertion(+)
+
+# diff --git a/README.md b/README.md
+# index 42290d5ce..dd432a768 100644
+# --- a/README.md
+# +++ b/README.md
+# @@ -809,6 +809,7 @@ The first argument can be either a `url` or an `options` object. The only requir
+#  - `followOriginalHttpMethod` - by default we redirect to HTTP method GET. you can enable this property to redirect to the original HTTP method (default: `false`)
+#  - `maxRedirects` - the maximum number of redirects to follow (default: `10`)
+#  - `removeRefererHeader` - removes the referer header when a redirect happens (default: `false`). **Note:** if true, referer header set in the initial request is preserved during redirect chain.
+# +- `allowInsecureRedirect` - allows cross-protocol redirects (HTTP to HTTPS and vice versa). **Warning:** may lead to bypassing anti SSRF filters
+
+#  ---
+
+
+# From 8a15249d182e54a261b1539846f76d913a6904f4 Mon Sep 17 00:00:00 2001
+# From: SzymonDrosdzol <84710686+SzymonDrosdzol@users.noreply.github.com>
+# Date: Fri, 17 Mar 2023 10:09:46 +0100
+# Subject: [PATCH 3/5] Removed semicolon
+
+# Co-authored-by: legobeat <109787230+legobeat@users.noreply.github.com>
+# ---
+#  lib/redirect.js | 2 +-
+#  1 file changed, 1 insertion(+), 1 deletion(-)
+
+# diff --git a/lib/redirect.js b/lib/redirect.js
+# index 770c7f41b..2864f9f2a 100644
+# --- a/lib/redirect.js
+# +++ b/lib/redirect.js
+# @@ -42,7 +42,7 @@ Redirect.prototype.onRequest = function (options) {
+#      self.followOriginalHttpMethod = options.followOriginalHttpMethod
+#    }
+#    if (options.allowInsecureRedirect !== undefined) {
+# -    self.allowInsecureRedirect = options.allowInsecureRedirect;
+# +    self.allowInsecureRedirect = options.allowInsecureRedirect
+#    }
+#  }
+
+
+# From 8535868fc88f24ed652d3f290bfd553a2cdbb811 Mon Sep 17 00:00:00 2001
+# From: SzymonDrosdzol <84710686+SzymonDrosdzol@users.noreply.github.com>
+# Date: Fri, 17 Mar 2023 10:12:09 +0100
+# Subject: [PATCH 4/5] Code style fix
+
+# Co-authored-by: Kevin van Rijn <6368561+kevinvanrijn@users.noreply.github.com>
+# ---
+#  tests/test-redirect.js | 2 +-
+#  1 file changed, 1 insertion(+), 1 deletion(-)
+
+# diff --git a/tests/test-redirect.js b/tests/test-redirect.js
+# index 48b4982e4..3e1957604 100644
+# --- a/tests/test-redirect.js
+# +++ b/tests/test-redirect.js
+# @@ -362,7 +362,7 @@ tape('http to https redirect should fail without the explicit "allowInsecureRedi
+#      rejectUnauthorized: false
+#    }, function (err, res, body) {
+#      t.notEqual(err, null)
+# -    t.equal(err.code, "ERR_INVALID_PROTOCOL","Failed to cross-protocol redirect")
+# +    t.equal(err.code, 'ERR_INVALID_PROTOCOL', 'Failed to cross-protocol redirect')
+#      t.end()
+#    })
+#  })
+
+# From 43647c4bd6e451f350267d5236463b4248dbc8df Mon Sep 17 00:00:00 2001
+# From: SzymonDrosdzol <84710686+SzymonDrosdzol@users.noreply.github.com>
+# Date: Fri, 17 Mar 2023 10:22:33 +0100
+# Subject: [PATCH 5/5] Removed leftover declaration
+
+# ---
+#  tests/test-httpModule.js | 1 -
+#  1 file changed, 1 deletion(-)
+
+# diff --git a/tests/test-httpModule.js b/tests/test-httpModule.js
+# index a59c427b1..f12382fe6 100644
+# --- a/tests/test-httpModule.js
+# +++ b/tests/test-httpModule.js
+# @@ -71,7 +71,6 @@ function runTests (name, httpModules) {
+#      var toHttps = 'http://localhost:' + plainServer.port + '/to_https'
+#      var toPlain = 'https://localhost:' + httpsServer.port + '/to_plain'
+#      var options = { httpModules: httpModules, strictSSL: false, allowInsecureRedirect: true }
+# -    var optionsSecure = { httpModules: httpModules, strictSSL: false }
+#      var modulesTest = httpModules || {}
+
+#      clearFauxRequests()
diff --git a/SPECS/nodejs/nodejs.spec b/SPECS/nodejs/nodejs.spec
@@ -1,18 +1,19 @@
 Summary:        A JavaScript runtime built on Chrome's V8 JavaScript engine.
 Name:           nodejs
 Version:        14.21.1
-Release:        1%{?dist}
+Release:        2%{?dist}
 License:        BSD and MIT and Public Domain and naist-2003
 Vendor:         Microsoft Corporation
 Distribution:   Mariner
 Group:          Applications/System
 URL:            https://github.com/nodejs/node
-# !!!! Nodejs code has a vendored version of OpenSSL code that must be removed from source tarball 
+# !!!! Nodejs code has a vendored version of OpenSSL code that must be removed from source tarball
 # !!!! because it contains patented algorithms.
 # !!!  => use clean-source-tarball.sh script to create a clean and reproducible source tarball.
 Source0:        https://nodejs.org/download/release/v%{version}/node-v%{version}.tar.xz
 Patch0:         patch_tls_nodejs14.patch
 Patch1:         remove_unsupported_tlsv13_ciphers.patch
+Patch2:         CVE-2023-28155.patch
 BuildRequires:  coreutils >= 8.22
 BuildRequires:  openssl-devel >= 1.1.1
 BuildRequires:  python3
@@ -79,6 +80,9 @@ make cctest
 %{_datadir}/systemtap/tapset/node.stp
 
 %changelog
+* Thu May 25 2023 Tobias Brick <tobiasb@microsoft.com> - 14.21.1-2
+- Add patch to fix CVE-2023-28155
+
 * Sun Dec 11 2022 CBL-Mariner Servicing Account <cblmargh@microsoft.com> - 14.21.1-1
 - Auto-upgrade to 14.21.1 - CVE-2022-3602_CVE-2022-3786_CVE-2022-43548