CVE-2023-38504: Insufficient Information (NVD-CWE-noinfo)
value: Sails is a realtime MVC Framework for Node.js. In Sails apps prior to version 1.5.7,, an attacker can send a virtual request that will cause the node process to crash. This behavior was fixed in Sails v1.5.7. As a workaround, disable the sockets hook and remove the `sails.io.js` client.
CWE description: There is insufficient information about the issue to classify it; details are unkown or unspecified.
CVSS v3 base score: 7.5
Published: 2023-07-27T19:15Z
Repository: https://github.com/balderdashy/sails
Language: JavaScript
Files changed: lib/router/req.js
Commit date: 2023-07-21 16:23:38 -0500
Commit message: Improve virtual request parsing (#7287)

* Improve virtual request parsing

Relevant for socket.io usage, and usage from tests.  (`sails.request()`)

* Update req.js

---------

Co-authored-by: Eric <eashaw@sailsjs.com>
Diff: diff --git a/lib/router/req.js b/lib/router/req.js
index d1a0036a..8efd64d1 100644
--- a/lib/router/req.js
+++ b/lib/router/req.js
@@ -44,26 +44,31 @@ module.exports = function buildRequest (_req) {
   if (typeof _req === 'object' && _req.read) {
     req = _req;
   }
   else {
 
-    // TODO: send a PR to mock-req with a fix for this
     if (_req.headers && typeof _req.headers === 'object') {
-      // Strip undefined headers
-      _.each(_req.headers, function (headerVal, headerKey) {
-        if (_.isUndefined(headerVal)){
+      for (let headerKey of Object.keys(_req.headers)) {
+        // Strip undefined headers
+        if (undefined === _req.headers[headerKey]) {
           delete _req.headers[headerKey];
         }
-      });
-      // Make sure all remaining headers are strings
-      _req.headers = _.mapValues(_req.headers, function (headerVal /*, headerKey*/) {
-        if (typeof headerVal !== 'string') {
-          headerVal = ''+headerVal+'';
+        // Make sure all remaining headers are strings
+        if (typeof _req.headers[headerKey] !== 'string') {
+          try {
+            _req.headers[headerKey] = ''+_req.headers[headerKey];
+            // FUTURE: This behavior is likely being relied upon by apps, so we can't just change it.
+            // But in retrospect, it would probably be better to straight-up reject this here if it's not
+            // a string, since HTTP header values are always supposed to be strings; or at least primitives.
+            // So maybe reject non-primitives, reject `null`, and then accept primitives, but be smart about
+            // this, especially in the context of what the client is doing.
+          } catch (unusedErr) {
+            delete _req.headers[headerKey];
+          }
         }
-        return headerVal;
-      });
-    }
+      }//∞
+    }//ﬁ
 
     // Create a mock IncomingMessage stream.
     req = new MockReq({
       method: _req && (_.isString(_req.method) ? _req.method.toUpperCase() : 'GET'),
       headers: _req && _req.headers || {},

Vulnerable code:     // TODO: send a PR to mock-req with a fix for this
      // Strip undefined headers
      _.each(_req.headers, function (headerVal, headerKey) {
        if (_.isUndefined(headerVal)){
      });
      // Make sure all remaining headers are strings
      _req.headers = _.mapValues(_req.headers, function (headerVal /*, headerKey*/) {
        if (typeof headerVal !== 'string') {
          headerVal = ''+headerVal+'';
        return headerVal;
      });
    }
Fixed code:       for (let headerKey of Object.keys(_req.headers)) {
        // Strip undefined headers
        if (undefined === _req.headers[headerKey]) {
        // Make sure all remaining headers are strings
        if (typeof _req.headers[headerKey] !== 'string') {
          try {
            _req.headers[headerKey] = ''+_req.headers[headerKey];
            // FUTURE: This behavior is likely being relied upon by apps, so we can't just change it.
            // But in retrospect, it would probably be better to straight-up reject this here if it's not
            // a string, since HTTP header values are always supposed to be strings; or at least primitives.
            // So maybe reject non-primitives, reject `null`, and then accept primitives, but be smart about
            // this, especially in the context of what the client is doing.
          } catch (unusedErr) {
            delete _req.headers[headerKey];
          }
      }//∞
    }//ﬁ