refactor: Remote code execution via MongoDB BSON parser through proto…

…type pollution; fixes security vulnerability [GHSA-462x-c3jw-7vr6](GHSA-462x-c3jw-7vr6) (#8677)
parse-community · Jun 28, 2023 · d6b17ba · d6b17ba
1 parent e212eb5
commit d6b17ba
Show file tree

Hide file tree

Showing 6 changed files with 101 additions and 34 deletions.
diff --git a/.eslintrc.json b/.eslintrc.json
@@ -25,5 +25,8 @@
         "space-infix-ops": "error",
         "no-useless-escape": "off",
         "require-atomic-updates": "off"
+    },
+    "globals": {
+        "Parse": true
     }
 }
diff --git a/spec/vulnerabilities.spec.js b/spec/vulnerabilities.spec.js
@@ -138,6 +138,71 @@ describe('Vulnerabilities', () => {
       );
     });
 
+    it('denies creating global config with polluted data', async () => {
+      const headers = {
+        'Content-Type': 'application/json',
+        'X-Parse-Application-Id': 'test',
+        'X-Parse-Master-Key': 'test',
+      };
+      const params = {
+        method: 'PUT',
+        url: 'http://localhost:8378/1/config',
+        json: true,
+        body: {
+          params: {
+            welcomeMesssage: 'Welcome to Parse',
+            foo: { _bsontype: 'Code', code: 'shell' },
+          },
+        },
+        headers,
+      };
+      const response = await request(params).catch(e => e);
+      expect(response.status).toBe(400);
+      const text = JSON.parse(response.text);
+      expect(text.code).toBe(Parse.Error.INVALID_KEY_NAME);
+      expect(text.error).toBe(
+        'Prohibited keyword in request data: {"key":"_bsontype","value":"Code"}.'
+      );
+    });
+
+    it('denies direct database write wih prohibited keys', async () => {
+      const Config = require('../lib/Config');
+      const config = Config.get(Parse.applicationId);
+      const user = {
+        objectId: '1234567890',
+        username: 'hello',
+        password: 'pass',
+        _session_token: 'abc',
+        foo: { _bsontype: 'Code', code: 'shell' },
+      };
+      await expectAsync(config.database.create('_User', user)).toBeRejectedWith(
+        new Parse.Error(
+          Parse.Error.INVALID_KEY_NAME,
+          'Prohibited keyword in request data: {"key":"_bsontype","value":"Code"}.'
+        )
+      );
+    });
+
+    it('denies direct database update wih prohibited keys', async () => {
+      const Config = require('../lib/Config');
+      const config = Config.get(Parse.applicationId);
+      const user = {
+        objectId: '1234567890',
+        username: 'hello',
+        password: 'pass',
+        _session_token: 'abc',
+        foo: { _bsontype: 'Code', code: 'shell' },
+      };
+      await expectAsync(
+        config.database.update('_User', { _id: user.objectId }, user)
+      ).toBeRejectedWith(
+        new Parse.Error(
+          Parse.Error.INVALID_KEY_NAME,
+          'Prohibited keyword in request data: {"key":"_bsontype","value":"Code"}.'
+        )
+      );
+    });
+
     it('denies creating a hook with polluted data', async () => {
       const express = require('express');
       const bodyParser = require('body-parser');

diff --git a/src/Controllers/DatabaseController.js b/src/Controllers/DatabaseController.js
@@ -475,6 +475,11 @@ class DatabaseController {
     validateOnly: boolean = false,
     validSchemaController: SchemaController.SchemaController
   ): Promise<any> {
+    try {
+      Utils.checkProhibitedKeywords(this.options, update);
+    } catch (error) {
+      return Promise.reject(new Parse.Error(Parse.Error.INVALID_KEY_NAME, error));
+    }
     const originalQuery = query;
     const originalUpdate = update;
     // Make a copy of the object, so we don't mutate the incoming data.
@@ -805,6 +810,11 @@ class DatabaseController {
     validateOnly: boolean = false,
     validSchemaController: SchemaController.SchemaController
   ): Promise<any> {
+    try {
+      Utils.checkProhibitedKeywords(this.options, object);
+    } catch (error) {
+      return Promise.reject(new Parse.Error(Parse.Error.INVALID_KEY_NAME, error));
+    }
     // Make a copy of the object, so we don't mutate the incoming data.
     const originalObject = object;
     object = transformObjectACL(object);

diff --git a/src/RestWrite.js b/src/RestWrite.js
@@ -64,8 +64,6 @@ function RestWrite(config, auth, className, query, data, originalData, clientSDK
     }
   }
 
-  this.checkProhibitedKeywords(data);
-
   // When the operation is complete, this.response may have several
   // fields.
   // response: the actual data to be returned
@@ -304,7 +302,11 @@ RestWrite.prototype.runBeforeSaveTrigger = function () {
           delete this.data.objectId;
         }
       }
-      this.checkProhibitedKeywords(this.data);
+      try {
+        Utils.checkProhibitedKeywords(this.config, this.data);
+      } catch (error) {
+        throw new Parse.Error(Parse.Error.INVALID_KEY_NAME, error);
+      }
     });
 };
 
@@ -1797,20 +1799,5 @@ RestWrite.prototype._updateResponseWithData = function (response, data) {
   return response;
 };
 
-RestWrite.prototype.checkProhibitedKeywords = function (data) {
-  if (this.config.requestKeywordDenylist) {
-    // Scan request data for denied keywords
-    for (const keyword of this.config.requestKeywordDenylist) {
-      const match = Utils.objectContainsKeyValue(data, keyword.key, keyword.value);
-      if (match) {
-        throw new Parse.Error(
-          Parse.Error.INVALID_KEY_NAME,
-          `Prohibited keyword in request data: ${JSON.stringify(keyword)}.`
-        );
-      }
-    }
-  }
-};
-
 export default RestWrite;
 module.exports = RestWrite;
diff --git a/src/Routers/FilesRouter.js b/src/Routers/FilesRouter.js
@@ -175,22 +175,12 @@ export class FilesRouter {
     const base64 = req.body.toString('base64');
     const file = new Parse.File(filename, { base64 }, contentType);
     const { metadata = {}, tags = {} } = req.fileData || {};
-    if (req.config && req.config.requestKeywordDenylist) {
-      // Scan request data for denied keywords
-      for (const keyword of req.config.requestKeywordDenylist) {
-        const match =
-          Utils.objectContainsKeyValue(metadata, keyword.key, keyword.value) ||
-          Utils.objectContainsKeyValue(tags, keyword.key, keyword.value);
-        if (match) {
-          next(
-            new Parse.Error(
-              Parse.Error.INVALID_KEY_NAME,
-              `Prohibited keyword in request data: ${JSON.stringify(keyword)}.`
-            )
-          );
-          return;
-        }
-      }
+    try {
+      Utils.checkProhibitedKeywords(config, metadata);
+      Utils.checkProhibitedKeywords(config, tags);
+    } catch (error) {
+      next(new Parse.Error(Parse.Error.INVALID_KEY_NAME, error));
+      return;
     }
     file.setTags(tags);
     file.setMetadata(metadata);

diff --git a/src/Utils.js b/src/Utils.js
@@ -358,6 +358,18 @@ class Utils {
     }
     return false;
   }
+
+  static checkProhibitedKeywords(config, data) {
+    if (config?.requestKeywordDenylist) {
+      // Scan request data for denied keywords
+      for (const keyword of config.requestKeywordDenylist) {
+        const match = Utils.objectContainsKeyValue(data, keyword.key, keyword.value);
+        if (match) {
+          throw `Prohibited keyword in request data: ${JSON.stringify(keyword)}.`;
+        }
+      }
+    }
+  }
 }
 
 module.exports = Utils;