From 5ad2c712b58d13bbf040a993df06f451661e40ac Mon Sep 17 00:00:00 2001 From: Kartal Kaan Bozdogan Date: Thu, 26 Sep 2024 23:39:45 +0200 Subject: [PATCH 01/11] Fix GHSA-8xq9-g7ch-35hg/CVE-2024-47183 by checking _User objects' custom object ids --- spec/vulnerabilities.spec.js | 11 +++++++++++ src/Routers/ClassesRouter.js | 7 +++++++ 2 files changed, 18 insertions(+) diff --git a/spec/vulnerabilities.spec.js b/spec/vulnerabilities.spec.js index 5017fe7230..0edab3d6c7 100644 --- a/spec/vulnerabilities.spec.js +++ b/spec/vulnerabilities.spec.js @@ -1,6 +1,17 @@ const request = require('../lib/request'); describe('Vulnerabilities', () => { + describe('(GHSA-8xq9-g7ch-35hg) ACL user id/role name confusion', () => { + beforeAll(async () => { + await reconfigureServer({ allowCustomObjectId: true }); + Parse.allowCustomObjectId = true; + }); + it('denies user creation with malicious object id', async () => { + await expectAsync( + new Parse.User({ id: 'role:a', username: 'a', password: '123' }).save() + ).toBeRejectedWith(new Parse.Error(Parse.Error.OPERATION_FORBIDDEN, 'Invalid user id.')); + }); + }); describe('Object prototype pollution', () => { it('denies object prototype to be polluted with keyword "constructor"', async () => { const headers = { diff --git a/src/Routers/ClassesRouter.js b/src/Routers/ClassesRouter.js index 0a68e25254..64168ddf18 100644 --- a/src/Routers/ClassesRouter.js +++ b/src/Routers/ClassesRouter.js @@ -106,6 +106,13 @@ export class ClassesRouter extends PromiseRouter { } handleCreate(req) { + if ( + this.className(req) === '_User' && + typeof req.body?.objectId === 'string' && + req.body.objectId.startsWith('role:') + ) { + throw new Parse.Error(Parse.Error.OPERATION_FORBIDDEN, 'Invalid user id.'); + } return rest.create( req.config, req.auth, From 5663d59494e0f906b250703ae525b21c077f310a Mon Sep 17 00:00:00 2001 From: Kartal Kaan Bozdogan Date: Sat, 28 Sep 2024 15:54:10 +0200 Subject: [PATCH 02/11] Refuse to honor the existing sessions for users with poisoned object ids --- spec/vulnerabilities.spec.js | 24 ++++++++++++++++++++++++ src/Auth.js | 3 +++ 2 files changed, 27 insertions(+) diff --git a/spec/vulnerabilities.spec.js b/spec/vulnerabilities.spec.js index 0edab3d6c7..a62427969e 100644 --- a/spec/vulnerabilities.spec.js +++ b/spec/vulnerabilities.spec.js @@ -11,6 +11,30 @@ describe('Vulnerabilities', () => { new Parse.User({ id: 'role:a', username: 'a', password: '123' }).save() ).toBeRejectedWith(new Parse.Error(Parse.Error.OPERATION_FORBIDDEN, 'Invalid user id.')); }); + describe('existing sessions for users with poisoned ids', () => { + /** @type {Parse.User} */ + let poisonedUser; + /** @type {Parse.User} */ + let innocentUser; + beforeAll(async () => { + const parseServer = await global.reconfigureServer(); + const databaseController = parseServer.config.databaseController; + [poisonedUser, innocentUser] = await Promise.all( + ['role:abc', 'abc'].map(async id => { + // Create the users directly on the db to bypass the user creation check + await databaseController.create('_User', { objectId: id }); + // Use the master key to create a session for them to bypass the session check + return Parse.User.loginAs(id); + }) + ); + }); + it('are refused', async () => { + await expectAsync( + new Parse.Query(Parse.User).find({ sessionToken: poisonedUser.getSessionToken() }) + ).toBeRejectedWith(new Parse.Error(Parse.Error.INTERNAL_SERVER_ERROR, 'Invalid user id.')); + await new Parse.Query(Parse.User).find({ sessionToken: innocentUser.getSessionToken() }); + }); + }); }); describe('Object prototype pollution', () => { it('denies object prototype to be polluted with keyword "constructor"', async () => { diff --git a/src/Auth.js b/src/Auth.js index c5351fe9fc..3c2848257b 100644 --- a/src/Auth.js +++ b/src/Auth.js @@ -180,6 +180,9 @@ const getAuthForSessionToken = async function ({ throw new Parse.Error(Parse.Error.INVALID_SESSION_TOKEN, 'Session token is expired.'); } const obj = session.user; + if (typeof obj['objectId'] === 'string' && obj['objectId'].startsWith('role:')) { + throw new Parse.Error(Parse.Error.INTERNAL_SERVER_ERROR, 'Invalid user id.'); + } delete obj.password; obj['className'] = '_User'; obj['sessionToken'] = sessionToken; From eeb0dc499b9bd14a1ae5ef59ddba9eb60d104ec6 Mon Sep 17 00:00:00 2001 From: Manuel <5673677+mtrezza@users.noreply.github.com> Date: Thu, 3 Oct 2024 19:03:10 +0200 Subject: [PATCH 03/11] minor style fixes Signed-off-by: Manuel <5673677+mtrezza@users.noreply.github.com> --- spec/vulnerabilities.spec.js | 5 ++++- src/Auth.js | 2 ++ src/Routers/ClassesRouter.js | 2 +- 3 files changed, 7 insertions(+), 2 deletions(-) diff --git a/spec/vulnerabilities.spec.js b/spec/vulnerabilities.spec.js index a62427969e..58346c6aaf 100644 --- a/spec/vulnerabilities.spec.js +++ b/spec/vulnerabilities.spec.js @@ -1,16 +1,18 @@ const request = require('../lib/request'); describe('Vulnerabilities', () => { - describe('(GHSA-8xq9-g7ch-35hg) ACL user id/role name confusion', () => { + describe('(GHSA-8xq9-g7ch-35hg) Custom object ID allows to acquire role privilege', () => { beforeAll(async () => { await reconfigureServer({ allowCustomObjectId: true }); Parse.allowCustomObjectId = true; }); + it('denies user creation with malicious object id', async () => { await expectAsync( new Parse.User({ id: 'role:a', username: 'a', password: '123' }).save() ).toBeRejectedWith(new Parse.Error(Parse.Error.OPERATION_FORBIDDEN, 'Invalid user id.')); }); + describe('existing sessions for users with poisoned ids', () => { /** @type {Parse.User} */ let poisonedUser; @@ -28,6 +30,7 @@ describe('Vulnerabilities', () => { }) ); }); + it('are refused', async () => { await expectAsync( new Parse.Query(Parse.User).find({ sessionToken: poisonedUser.getSessionToken() }) diff --git a/src/Auth.js b/src/Auth.js index 3c2848257b..00eab9112e 100644 --- a/src/Auth.js +++ b/src/Auth.js @@ -180,9 +180,11 @@ const getAuthForSessionToken = async function ({ throw new Parse.Error(Parse.Error.INVALID_SESSION_TOKEN, 'Session token is expired.'); } const obj = session.user; + if (typeof obj['objectId'] === 'string' && obj['objectId'].startsWith('role:')) { throw new Parse.Error(Parse.Error.INTERNAL_SERVER_ERROR, 'Invalid user id.'); } + delete obj.password; obj['className'] = '_User'; obj['sessionToken'] = sessionToken; diff --git a/src/Routers/ClassesRouter.js b/src/Routers/ClassesRouter.js index 64168ddf18..1f9f93e329 100644 --- a/src/Routers/ClassesRouter.js +++ b/src/Routers/ClassesRouter.js @@ -111,7 +111,7 @@ export class ClassesRouter extends PromiseRouter { typeof req.body?.objectId === 'string' && req.body.objectId.startsWith('role:') ) { - throw new Parse.Error(Parse.Error.OPERATION_FORBIDDEN, 'Invalid user id.'); + throw new Parse.Error(Parse.Error.OPERATION_FORBIDDEN, 'Invalid object ID.'); } return rest.create( req.config, From aa1a3430af2c302ac48c1e880c73c7d4fe5677a3 Mon Sep 17 00:00:00 2001 From: Manuel <5673677+mtrezza@users.noreply.github.com> Date: Thu, 3 Oct 2024 19:04:49 +0200 Subject: [PATCH 04/11] update error msg Signed-off-by: Manuel <5673677+mtrezza@users.noreply.github.com> --- spec/vulnerabilities.spec.js | 4 ++-- src/Auth.js | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/spec/vulnerabilities.spec.js b/spec/vulnerabilities.spec.js index 58346c6aaf..706fbe72ae 100644 --- a/spec/vulnerabilities.spec.js +++ b/spec/vulnerabilities.spec.js @@ -10,7 +10,7 @@ describe('Vulnerabilities', () => { it('denies user creation with malicious object id', async () => { await expectAsync( new Parse.User({ id: 'role:a', username: 'a', password: '123' }).save() - ).toBeRejectedWith(new Parse.Error(Parse.Error.OPERATION_FORBIDDEN, 'Invalid user id.')); + ).toBeRejectedWith(new Parse.Error(Parse.Error.OPERATION_FORBIDDEN, 'Invalid object ID.')); }); describe('existing sessions for users with poisoned ids', () => { @@ -34,7 +34,7 @@ describe('Vulnerabilities', () => { it('are refused', async () => { await expectAsync( new Parse.Query(Parse.User).find({ sessionToken: poisonedUser.getSessionToken() }) - ).toBeRejectedWith(new Parse.Error(Parse.Error.INTERNAL_SERVER_ERROR, 'Invalid user id.')); + ).toBeRejectedWith(new Parse.Error(Parse.Error.INTERNAL_SERVER_ERROR, 'Invalid object ID.')); await new Parse.Query(Parse.User).find({ sessionToken: innocentUser.getSessionToken() }); }); }); diff --git a/src/Auth.js b/src/Auth.js index 00eab9112e..3177597f70 100644 --- a/src/Auth.js +++ b/src/Auth.js @@ -182,7 +182,7 @@ const getAuthForSessionToken = async function ({ const obj = session.user; if (typeof obj['objectId'] === 'string' && obj['objectId'].startsWith('role:')) { - throw new Parse.Error(Parse.Error.INTERNAL_SERVER_ERROR, 'Invalid user id.'); + throw new Parse.Error(Parse.Error.INTERNAL_SERVER_ERROR, 'Invalid object ID.'); } delete obj.password; From 7ef4016b4ae56cc24623da1374c8ec7eeffc2e37 Mon Sep 17 00:00:00 2001 From: Manuel <5673677+mtrezza@users.noreply.github.com> Date: Thu, 3 Oct 2024 19:16:26 +0200 Subject: [PATCH 05/11] test description rephrase Signed-off-by: Manuel <5673677+mtrezza@users.noreply.github.com> --- spec/vulnerabilities.spec.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/spec/vulnerabilities.spec.js b/spec/vulnerabilities.spec.js index 706fbe72ae..288acfc570 100644 --- a/spec/vulnerabilities.spec.js +++ b/spec/vulnerabilities.spec.js @@ -7,13 +7,13 @@ describe('Vulnerabilities', () => { Parse.allowCustomObjectId = true; }); - it('denies user creation with malicious object id', async () => { + it('denies user creation with malicious object ID', async () => { await expectAsync( new Parse.User({ id: 'role:a', username: 'a', password: '123' }).save() ).toBeRejectedWith(new Parse.Error(Parse.Error.OPERATION_FORBIDDEN, 'Invalid object ID.')); }); - describe('existing sessions for users with poisoned ids', () => { + describe('existing sessions for users with poisoned object ID', () => { /** @type {Parse.User} */ let poisonedUser; /** @type {Parse.User} */ @@ -31,7 +31,7 @@ describe('Vulnerabilities', () => { ); }); - it('are refused', async () => { + it('refuses session token of user with poisoned object ID', async () => { await expectAsync( new Parse.Query(Parse.User).find({ sessionToken: poisonedUser.getSessionToken() }) ).toBeRejectedWith(new Parse.Error(Parse.Error.INTERNAL_SERVER_ERROR, 'Invalid object ID.')); From 3ca11caab64934885ef3ac6919a100650bb1c51c Mon Sep 17 00:00:00 2001 From: Manuel Trezza <5673677+mtrezza@users.noreply.github.com> Date: Thu, 3 Oct 2024 20:20:32 +0200 Subject: [PATCH 06/11] fix test --- spec/vulnerabilities.spec.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/vulnerabilities.spec.js b/spec/vulnerabilities.spec.js index 288acfc570..3b9cb8db32 100644 --- a/spec/vulnerabilities.spec.js +++ b/spec/vulnerabilities.spec.js @@ -24,7 +24,7 @@ describe('Vulnerabilities', () => { [poisonedUser, innocentUser] = await Promise.all( ['role:abc', 'abc'].map(async id => { // Create the users directly on the db to bypass the user creation check - await databaseController.create('_User', { objectId: id }); + await databaseController.create('_User', { objectId: id, username: id, password: '1' }); // Use the master key to create a session for them to bypass the session check return Parse.User.loginAs(id); }) From 3521fbabf606882a8d11b4aac0715eb7f2a01853 Mon Sep 17 00:00:00 2001 From: Manuel Trezza <5673677+mtrezza@users.noreply.github.com> Date: Thu, 3 Oct 2024 20:27:09 +0200 Subject: [PATCH 07/11] Update vulnerabilities.spec.js --- spec/vulnerabilities.spec.js | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/spec/vulnerabilities.spec.js b/spec/vulnerabilities.spec.js index 3b9cb8db32..9da81f7538 100644 --- a/spec/vulnerabilities.spec.js +++ b/spec/vulnerabilities.spec.js @@ -7,7 +7,7 @@ describe('Vulnerabilities', () => { Parse.allowCustomObjectId = true; }); - it('denies user creation with malicious object ID', async () => { + it('denies user creation with poisoned object ID', async () => { await expectAsync( new Parse.User({ id: 'role:a', username: 'a', password: '123' }).save() ).toBeRejectedWith(new Parse.Error(Parse.Error.OPERATION_FORBIDDEN, 'Invalid object ID.')); @@ -18,7 +18,8 @@ describe('Vulnerabilities', () => { let poisonedUser; /** @type {Parse.User} */ let innocentUser; - beforeAll(async () => { + + beforeEach(async () => { const parseServer = await global.reconfigureServer(); const databaseController = parseServer.config.databaseController; [poisonedUser, innocentUser] = await Promise.all( From 118fa6c67c68c83d436201d5426d220763c90499 Mon Sep 17 00:00:00 2001 From: Manuel Trezza <5673677+mtrezza@users.noreply.github.com> Date: Thu, 3 Oct 2024 20:39:38 +0200 Subject: [PATCH 08/11] Update vulnerabilities.spec.js --- spec/vulnerabilities.spec.js | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/spec/vulnerabilities.spec.js b/spec/vulnerabilities.spec.js index 9da81f7538..727dfccee1 100644 --- a/spec/vulnerabilities.spec.js +++ b/spec/vulnerabilities.spec.js @@ -7,6 +7,11 @@ describe('Vulnerabilities', () => { Parse.allowCustomObjectId = true; }); + afterAll(async () => { + await reconfigureServer({ allowCustomObjectId: false }); + Parse.allowCustomObjectId = false; + }); + it('denies user creation with poisoned object ID', async () => { await expectAsync( new Parse.User({ id: 'role:a', username: 'a', password: '123' }).save() @@ -19,7 +24,7 @@ describe('Vulnerabilities', () => { /** @type {Parse.User} */ let innocentUser; - beforeEach(async () => { + beforeAll(async () => { const parseServer = await global.reconfigureServer(); const databaseController = parseServer.config.databaseController; [poisonedUser, innocentUser] = await Promise.all( @@ -40,6 +45,7 @@ describe('Vulnerabilities', () => { }); }); }); + describe('Object prototype pollution', () => { it('denies object prototype to be polluted with keyword "constructor"', async () => { const headers = { From f7b36ef6a266e150a2375bf8455a649127167821 Mon Sep 17 00:00:00 2001 From: Manuel Trezza <5673677+mtrezza@users.noreply.github.com> Date: Thu, 3 Oct 2024 20:48:43 +0200 Subject: [PATCH 09/11] fix postgres tests --- spec/vulnerabilities.spec.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/vulnerabilities.spec.js b/spec/vulnerabilities.spec.js index 727dfccee1..5eb3c6cb93 100644 --- a/spec/vulnerabilities.spec.js +++ b/spec/vulnerabilities.spec.js @@ -30,7 +30,7 @@ describe('Vulnerabilities', () => { [poisonedUser, innocentUser] = await Promise.all( ['role:abc', 'abc'].map(async id => { // Create the users directly on the db to bypass the user creation check - await databaseController.create('_User', { objectId: id, username: id, password: '1' }); + await databaseController.create('_User', { objectId: id }); // Use the master key to create a session for them to bypass the session check return Parse.User.loginAs(id); }) From d598ace6149f1f580556d621b57b93a8463889cc Mon Sep 17 00:00:00 2001 From: Manuel Trezza <5673677+mtrezza@users.noreply.github.com> Date: Thu, 3 Oct 2024 20:48:58 +0200 Subject: [PATCH 10/11] fit --- spec/vulnerabilities.spec.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/vulnerabilities.spec.js b/spec/vulnerabilities.spec.js index 5eb3c6cb93..dc73203df0 100644 --- a/spec/vulnerabilities.spec.js +++ b/spec/vulnerabilities.spec.js @@ -1,6 +1,6 @@ const request = require('../lib/request'); -describe('Vulnerabilities', () => { +fdescribe('Vulnerabilities', () => { describe('(GHSA-8xq9-g7ch-35hg) Custom object ID allows to acquire role privilege', () => { beforeAll(async () => { await reconfigureServer({ allowCustomObjectId: true }); From cdcb2939a7e3db5105ec8e249da55470002c9957 Mon Sep 17 00:00:00 2001 From: Manuel Trezza <5673677+mtrezza@users.noreply.github.com> Date: Thu, 3 Oct 2024 20:50:47 +0200 Subject: [PATCH 11/11] unfit --- spec/vulnerabilities.spec.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/vulnerabilities.spec.js b/spec/vulnerabilities.spec.js index dc73203df0..5eb3c6cb93 100644 --- a/spec/vulnerabilities.spec.js +++ b/spec/vulnerabilities.spec.js @@ -1,6 +1,6 @@ const request = require('../lib/request'); -fdescribe('Vulnerabilities', () => { +describe('Vulnerabilities', () => { describe('(GHSA-8xq9-g7ch-35hg) Custom object ID allows to acquire role privilege', () => { beforeAll(async () => { await reconfigureServer({ allowCustomObjectId: true });