From 898681b6200548f1bab61e2bf8d400831f951de3 Mon Sep 17 00:00:00 2001 From: Vusala Alakbarova <58344468+vuusale@users.noreply.github.com> Date: Sun, 16 Jun 2024 09:57:00 +0400 Subject: [PATCH 1/4] fix: adding property existence check before referencing To prevent prototype pollution gadget, added a check to ensure that 'mongocryptSpawnPath' and 'mongocryptSpawnArgs' properties exist in 'extraOptions' object before assigning them to 'this.spawnPath' and 'this.spawnArgs', which are passed to 'spawn' function in line 53. HackerOne report: https://hackerone.com/bugs?subject=user&report_id=2522466 --- src/client-side-encryption/mongocryptd_manager.ts | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/client-side-encryption/mongocryptd_manager.ts b/src/client-side-encryption/mongocryptd_manager.ts index 499f2aab296..96f0a2d64ec 100644 --- a/src/client-side-encryption/mongocryptd_manager.ts +++ b/src/client-side-encryption/mongocryptd_manager.ts @@ -24,9 +24,11 @@ export class MongocryptdManager { this.bypassSpawn = !!extraOptions.mongocryptdBypassSpawn; - this.spawnPath = extraOptions.mongocryptdSpawnPath || ''; + if (Object.keys(extraOptions).includes('mongocryptdSpawnPath')) { + this.spawnPath = extraOptions.mongocryptdSpawnPath || ''; + } else this.spawnPath = ''; this.spawnArgs = []; - if (Array.isArray(extraOptions.mongocryptdSpawnArgs)) { + if (Object.keys(extraOptions).includes('mongocryptdSpawnArgs') && Array.isArray(extraOptions.mongocryptdSpawnArgs)) { this.spawnArgs = this.spawnArgs.concat(extraOptions.mongocryptdSpawnArgs); } if ( From 2a60bd1b102bf5ca1d648b88787418798b5af7f6 Mon Sep 17 00:00:00 2001 From: Durran Jordan Date: Thu, 27 Jun 2024 21:07:20 +0200 Subject: [PATCH 2/4] fix: check using hasOwn, add tests --- src/client-side-encryption/mongocryptd_manager.ts | 13 ++++++++----- .../mongocryptd_manager.test.ts | 10 ++++++++++ tsconfig.json | 3 ++- 3 files changed, 20 insertions(+), 6 deletions(-) diff --git a/src/client-side-encryption/mongocryptd_manager.ts b/src/client-side-encryption/mongocryptd_manager.ts index 96f0a2d64ec..4bfac2ed796 100644 --- a/src/client-side-encryption/mongocryptd_manager.ts +++ b/src/client-side-encryption/mongocryptd_manager.ts @@ -12,8 +12,8 @@ export class MongocryptdManager { uri: string; bypassSpawn: boolean; - spawnPath: string; - spawnArgs: Array; + spawnPath = ''; + spawnArgs: Array = []; _child?: ChildProcess; constructor(extraOptions: AutoEncryptionExtraOptions = {}) { @@ -24,11 +24,14 @@ export class MongocryptdManager { this.bypassSpawn = !!extraOptions.mongocryptdBypassSpawn; - if (Object.keys(extraOptions).includes('mongocryptdSpawnPath')) { + if (Object.hasOwn(extraOptions, 'mongocryptdSpawnPath')) { this.spawnPath = extraOptions.mongocryptdSpawnPath || ''; - } else this.spawnPath = ''; + } this.spawnArgs = []; - if (Object.keys(extraOptions).includes('mongocryptdSpawnArgs') && Array.isArray(extraOptions.mongocryptdSpawnArgs)) { + if ( + Object.hasOwn(extraOptions, 'mongocryptdSpawnArgs') && + Array.isArray(extraOptions.mongocryptdSpawnArgs) + ) { this.spawnArgs = this.spawnArgs.concat(extraOptions.mongocryptdSpawnArgs); } if ( diff --git a/test/unit/client-side-encryption/mongocryptd_manager.test.ts b/test/unit/client-side-encryption/mongocryptd_manager.test.ts index 8122841e3b5..2de59bf9d62 100644 --- a/test/unit/client-side-encryption/mongocryptd_manager.test.ts +++ b/test/unit/client-side-encryption/mongocryptd_manager.test.ts @@ -22,6 +22,16 @@ describe('MongocryptdManager', function () { expect(mcdm.spawnArgs).to.deep.equal(['--idleShutdownTimeoutSecs', '12']); }); + it('does not allow prototype pollution on spawn path', function () { + const mcdm = new MongocryptdManager({ __proto__: { mongocryptdSpawnPath: 'test' } }); + expect(mcdm.spawnPath).to.equal(''); + }); + + it('does not allow prototype pollution on spawn args', function () { + const mcdm = new MongocryptdManager({ __proto__: { mongocryptdSpawnArgs: 'test' } }); + expect(mcdm.spawnArgs).to.deep.equal(['--idleShutdownTimeoutSecs', '60']); + }); + it('should not override `idleShutdownTimeoutSecs` if the user sets it using `key=value` form', function () { const mcdm = new MongocryptdManager({ mongocryptdSpawnArgs: ['--idleShutdownTimeoutSecs=12'] diff --git a/tsconfig.json b/tsconfig.json index 0d08d129980..942a6b869ad 100644 --- a/tsconfig.json +++ b/tsconfig.json @@ -10,7 +10,8 @@ "skipLibCheck": true, "lib": [ "es2021", - "ES2022.Error" + "ES2022.Error", + "ES2022.Object" ], // We don't make use of tslib helpers, all syntax used is supported by target engine "importHelpers": false, From b7743aa8c08c3c5510f4fc447dbb305bb7a3b9b2 Mon Sep 17 00:00:00 2001 From: Durran Jordan Date: Thu, 27 Jun 2024 21:12:34 +0200 Subject: [PATCH 3/4] test: use array --- test/unit/client-side-encryption/mongocryptd_manager.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/unit/client-side-encryption/mongocryptd_manager.test.ts b/test/unit/client-side-encryption/mongocryptd_manager.test.ts index 2de59bf9d62..f8004781d8c 100644 --- a/test/unit/client-side-encryption/mongocryptd_manager.test.ts +++ b/test/unit/client-side-encryption/mongocryptd_manager.test.ts @@ -28,7 +28,7 @@ describe('MongocryptdManager', function () { }); it('does not allow prototype pollution on spawn args', function () { - const mcdm = new MongocryptdManager({ __proto__: { mongocryptdSpawnArgs: 'test' } }); + const mcdm = new MongocryptdManager({ __proto__: { mongocryptdSpawnArgs: ['test'] } }); expect(mcdm.spawnArgs).to.deep.equal(['--idleShutdownTimeoutSecs', '60']); }); From 9e77586e6a5b253a66dc5bfe664f540511cd9f7e Mon Sep 17 00:00:00 2001 From: Durran Jordan Date: Mon, 1 Jul 2024 16:56:26 +0200 Subject: [PATCH 4/4] fix: dont default multiple times --- src/client-side-encryption/mongocryptd_manager.ts | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/client-side-encryption/mongocryptd_manager.ts b/src/client-side-encryption/mongocryptd_manager.ts index 4bfac2ed796..948d2410aa6 100644 --- a/src/client-side-encryption/mongocryptd_manager.ts +++ b/src/client-side-encryption/mongocryptd_manager.ts @@ -24,10 +24,9 @@ export class MongocryptdManager { this.bypassSpawn = !!extraOptions.mongocryptdBypassSpawn; - if (Object.hasOwn(extraOptions, 'mongocryptdSpawnPath')) { - this.spawnPath = extraOptions.mongocryptdSpawnPath || ''; + if (Object.hasOwn(extraOptions, 'mongocryptdSpawnPath') && extraOptions.mongocryptdSpawnPath) { + this.spawnPath = extraOptions.mongocryptdSpawnPath; } - this.spawnArgs = []; if ( Object.hasOwn(extraOptions, 'mongocryptdSpawnArgs') && Array.isArray(extraOptions.mongocryptdSpawnArgs)