CVE-2022-24719: Improper Removal of Sensitive Information Before Storage or Transfer (CWE-212)
value: Fluture-Node is a FP-style HTTP and streaming utils for Node based on Fluture. Using `followRedirects` or `followRedirectsWith` with any of the redirection strategies built into fluture-node 4.0.0 or 4.0.1, paired with a request that includes confidential headers such as Authorization or Cookie, exposes you to a vulnerability where, if the destination server were to redirect the request to a server on a third-party domain, or the same domain over unencrypted HTTP, the headers would be included in the follow-up request and be exposed to the third party, or potential http traffic sniffing. The redirection strategies made available in version 4.0.2 automatically redact confidential headers when a redirect is followed across to another origin. A workaround has been identified by using a custom redirection strategy via the `followRedirectsWith` function. The custom strategy can be based on the new strategies available in fluture-node@4.0.2.
CWE description: The product stores, transfers, or shares a resource that contains sensitive information, but it does not properly remove that information before the product makes the resource available to unauthorized actors.
CVSS v3 base score: 6.1
Published: 2022-03-01T21:15Z
Repository: https://github.com/fluture-js/fluture-node
Language: JavaScript
Files changed: index.js, test/index.js
Commit date: 2022-03-01 09:56:31 +0100
Commit message: Update redirection strategies to drop confidential headers

Diff: diff --git a/index.js b/index.js
index 2fe20b4..08eca2c 100644
--- a/index.js
+++ b/index.js
@@ -532,50 +532,73 @@ export const sendForm = method => url => headers => form => {
 export const matchStatus = f => fs => res => {
   const {statusCode} = Response.message (res);
   return (hasProp (statusCode) (fs) ? fs[statusCode] : f) (res);
 };
 
-// mergeUrls :: Url -> Any -> String
-const mergeUrls = base => input => (
+// mergeUrls :: (Url, Any) -> String
+const mergeUrls = (base, input) => (
   typeof input === 'string' ?
   new URL (input, base).href :
   base
 );
 
+// sameHost :: (Url, Url) -> Boolean
+const sameHost = (parent, child) => {
+  const p = new URL (parent);
+  const c = new URL (child);
+  return p.host === c.host || c.host.endsWith ('.' + p.host);
+};
+
 // overHeaders :: (Request, Array2 String String -> Array2 String String)
 //             -> Request
 const overHeaders = (request, f) => {
   const options = cleanRequestOptions (request);
   const headers = Object.fromEntries (f (Object.entries (options.headers)));
   return Request (Object.assign ({}, Request.options (request), {headers}))
                  (Request.url (request))
                  (Request.body (request));
 };
 
+// confidentialHeaders :: Array String
+const confidentialHeaders = [
+  'authorization',
+  'cookie',
+];
+
 //# redirectAnyRequest :: Response -> Request
 //.
 //. A redirection strategy that simply reissues the original Request to the
 //. Location specified in the given Response.
 //.
+//. If the new location is on an external host, then any confidential headers
+//. (such as the cookie header) will be dropped from the new request.
+//.
 //. Used in the [`defaultRedirectionPolicy`](#defaultRedirectionPolicy) and
 //. the [`aggressiveRedirectionPolicy`](#aggressiveRedirectionPolicy).
 export const redirectAnyRequest = response => {
   const {headers: {location}} = Response.message (response);
   const original = Response.request (response);
   const oldUrl = Request.url (original);
-  const newUrl = mergeUrls (oldUrl) (location);
-  return (Request (Request.options (original))
-                  (newUrl)
-                  (Request.body (original)));
+  const newUrl = mergeUrls (oldUrl, location);
+  const request = Request (Request.options (original))
+                          (newUrl)
+                          (Request.body (original));
+
+  return sameHost (oldUrl, newUrl) ? request : overHeaders (request, xs => (
+    xs.filter (([name]) => !confidentialHeaders.includes (name.toLowerCase ()))
+  ));
 };
 
 //# redirectIfGetMethod :: Response -> Request
 //.
 //. A redirection strategy that simply reissues the original Request to the
 //. Location specified in the given Response, but only if the original request
 //. was using the GET method.
 //.
+//. If the new location is on an external host, then any confidential headers
+//. (such as the cookie header) will be dropped from the new request.
+//.
 //. Used in [`followRedirectsStrict`](#followRedirectsStrict).
 export const redirectIfGetMethod = response => {
   const {method} = cleanRequestOptions (Response.request (response));
   return (
     method === 'GET' ?
@@ -588,12 +611,14 @@ export const redirectIfGetMethod = response => {
 //.
 //. A redirection strategy that sends a new GET request based on the original
 //. request to the Location specified in the given Response. If the response
 //. does not contain a valid location, the request is not redirected.
 //.
-//. The original request method and body are discarded, but all the options
-//. are preserved.
+//. The original request method and body are discarded, but other options
+//. are preserved. If the new location is on an external host, then any
+//. confidential headers (such as the cookie header) will be dropped from the
+//. new request.
 //.
 //. Used in the [`defaultRedirectionPolicy`](#defaultRedirectionPolicy) and
 //. the [`aggressiveRedirectionPolicy`](#aggressiveRedirectionPolicy).
 export const redirectUsingGetMethod = response => {
   const original = Response.request (response);
diff --git a/test/index.js b/test/index.js
index e211cf4..9ef837b 100644
--- a/test/index.js
+++ b/test/index.js
@@ -257,33 +257,53 @@ test ('sendForm', () => Promise.all ([
 
 test ('redirectAnyRequest', () => Promise.all ([
   assertResolves (fl.map (fn.redirectAnyRequest) (mockResponse ({})))
                  (getRequest),
   assertResolves (fl.map (fn.redirectAnyRequest) (getResponse (301) ('ftp://xxx')))
-                 (fn.Request ({}) ('ftp://xxx/') (fn.emptyStream)),
+                 (fn.Request ({headers: {}}) ('ftp://xxx/') (fn.emptyStream)),
   assertResolves (fl.map (fn.redirectAnyRequest) (getResponse (301) ('/echo')))
                  (fn.Request ({}) ('https://example.com/echo') (fn.emptyStream)),
   assertResolves (fl.map (fn.redirectAnyRequest) (postResponse (301) ('/echo')))
                  (fn.Request ({method: 'POST'}) ('https://example.com/echo') (fn.streamOf (Buffer.from ('test')))),
+  assertResolves (fl.map (fn.redirectAnyRequest)
+                         (mockResponse ({code: 301,
+                                         headers: {location: 'https://example.com/path'},
+                                         request: fn.Request ({headers: {cookie: 'yum'}}) ('https://example.com') (fn.emptyStream)})))
+                 (fn.Request ({headers: {cookie: 'yum'}}) ('https://example.com/path') (fn.emptyStream)),
+  assertResolves (fl.map (fn.redirectAnyRequest)
+                         (mockResponse ({code: 301,
+                                         headers: {location: 'https://sub.example.com/'},
+                                         request: fn.Request ({headers: {cookie: 'yum'}}) ('https://example.com') (fn.emptyStream)})))
+                 (fn.Request ({headers: {cookie: 'yum'}}) ('https://sub.example.com/') (fn.emptyStream)),
+  assertResolves (fl.map (fn.redirectAnyRequest)
+                         (mockResponse ({code: 301,
+                                         headers: {location: 'https://bigsub.example.com/'},
+                                         request: fn.Request ({headers: {cookie: 'yum'}}) ('https://example.com') (fn.emptyStream)})))
+                 (fn.Request ({headers: {cookie: 'yum'}}) ('https://bigsub.example.com/') (fn.emptyStream)),
+  assertResolves (fl.map (fn.redirectAnyRequest)
+                         (mockResponse ({code: 301,
+                                         headers: {location: 'https://elsewhere.com/'},
+                                         request: fn.Request ({headers: {cookie: 'yum'}}) ('https://example.com') (fn.emptyStream)})))
+                 (fn.Request ({headers: {}}) ('https://elsewhere.com/') (fn.emptyStream)),
 ]));
 
 test ('redirectIfGetMethod', () => Promise.all ([
   assertResolves (fl.map (fn.redirectIfGetMethod) (mockResponse ({})))
                  (getRequest),
   assertResolves (fl.map (fn.redirectIfGetMethod) (getResponse (301) ('ftp://xxx')))
-                 (fn.Request ({}) ('ftp://xxx/') (fn.emptyStream)),
+                 (fn.Request ({headers: {}}) ('ftp://xxx/') (fn.emptyStream)),
   assertResolves (fl.map (fn.redirectIfGetMethod) (getResponse (301) ('/echo')))
                  (fn.Request ({}) ('https://example.com/echo') (fn.emptyStream)),
   assertResolves (fl.map (fn.redirectIfGetMethod) (postResponse (301) ('/echo')))
                  (postRequest),
 ]));
 
 test ('redirectUsingGetMethod', () => Promise.all ([
   assertResolves (fl.map (fn.redirectUsingGetMethod) (mockResponse ({})))
                  (fn.Request ({method: 'GET'}) ('https://example.com') (fn.emptyStream)),
   assertResolves (fl.map (fn.redirectUsingGetMethod) (getResponse (301) ('ftp://xxx')))
-                 (fn.Request ({method: 'GET'}) ('ftp://xxx/') (fn.emptyStream)),
+                 (fn.Request ({method: 'GET', headers: {}}) ('ftp://xxx/') (fn.emptyStream)),
   assertResolves (fl.map (fn.redirectUsingGetMethod) (getResponse (200) ('/echo')))
                  (fn.Request ({method: 'GET'}) ('https://example.com/echo') (fn.emptyStream)),
   assertResolves (fl.map (fn.redirectUsingGetMethod) (postResponse (200) ('/echo')))
                  (fn.Request ({method: 'GET'}) ('https://example.com/echo') (fn.emptyStream)),
 ]));

Vulnerable code: // mergeUrls :: Url -> Any -> String
const mergeUrls = base => input => (
  const newUrl = mergeUrls (oldUrl) (location);
  return (Request (Request.options (original))
                  (newUrl)
                  (Request.body (original)));
//. The original request method and body are discarded, but all the options
//. are preserved.
                 (fn.Request ({}) ('ftp://xxx/') (fn.emptyStream)),
                 (fn.Request ({}) ('ftp://xxx/') (fn.emptyStream)),
                 (fn.Request ({method: 'GET'}) ('ftp://xxx/') (fn.emptyStream)),
Fixed code: // mergeUrls :: (Url, Any) -> String
const mergeUrls = (base, input) => (
// sameHost :: (Url, Url) -> Boolean
const sameHost = (parent, child) => {
  const p = new URL (parent);
  const c = new URL (child);
  return p.host === c.host || c.host.endsWith ('.' + p.host);
};

// confidentialHeaders :: Array String
const confidentialHeaders = [
  'authorization',
  'cookie',
];

//. If the new location is on an external host, then any confidential headers
//. (such as the cookie header) will be dropped from the new request.
//.
  const newUrl = mergeUrls (oldUrl, location);
  const request = Request (Request.options (original))
                          (newUrl)
                          (Request.body (original));

  return sameHost (oldUrl, newUrl) ? request : overHeaders (request, xs => (
    xs.filter (([name]) => !confidentialHeaders.includes (name.toLowerCase ()))
  ));
//. If the new location is on an external host, then any confidential headers
//. (such as the cookie header) will be dropped from the new request.
//.
//. The original request method and body are discarded, but other options
//. are preserved. If the new location is on an external host, then any
//. confidential headers (such as the cookie header) will be dropped from the
//. new request.
                 (fn.Request ({headers: {}}) ('ftp://xxx/') (fn.emptyStream)),
  assertResolves (fl.map (fn.redirectAnyRequest)
                         (mockResponse ({code: 301,
                                         headers: {location: 'https://example.com/path'},
                                         request: fn.Request ({headers: {cookie: 'yum'}}) ('https://example.com') (fn.emptyStream)})))
                 (fn.Request ({headers: {cookie: 'yum'}}) ('https://example.com/path') (fn.emptyStream)),
  assertResolves (fl.map (fn.redirectAnyRequest)
                         (mockResponse ({code: 301,
                                         headers: {location: 'https://sub.example.com/'},
                                         request: fn.Request ({headers: {cookie: 'yum'}}) ('https://example.com') (fn.emptyStream)})))
                 (fn.Request ({headers: {cookie: 'yum'}}) ('https://sub.example.com/') (fn.emptyStream)),
  assertResolves (fl.map (fn.redirectAnyRequest)
                         (mockResponse ({code: 301,
                                         headers: {location: 'https://bigsub.example.com/'},
                                         request: fn.Request ({headers: {cookie: 'yum'}}) ('https://example.com') (fn.emptyStream)})))
                 (fn.Request ({headers: {cookie: 'yum'}}) ('https://bigsub.example.com/') (fn.emptyStream)),
  assertResolves (fl.map (fn.redirectAnyRequest)
                         (mockResponse ({code: 301,
                                         headers: {location: 'https://elsewhere.com/'},
                                         request: fn.Request ({headers: {cookie: 'yum'}}) ('https://example.com') (fn.emptyStream)})))
                 (fn.Request ({headers: {}}) ('https://elsewhere.com/') (fn.emptyStream)),
                 (fn.Request ({headers: {}}) ('ftp://xxx/') (fn.emptyStream)),
                 (fn.Request ({method: 'GET', headers: {}}) ('ftp://xxx/') (fn.emptyStream)),