From acd980e820e9d20df545367617b7b6812142b380 Mon Sep 17 00:00:00 2001 From: EhsanParsania <75175231+EhsanParsania@users.noreply.github.com> Date: Wed, 13 Mar 2024 21:11:37 +0400 Subject: [PATCH 1/6] fix server crash on invalid Cloud Function call --- src/triggers.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/triggers.js b/src/triggers.js index bfebde2e83..13405209f2 100644 --- a/src/triggers.js +++ b/src/triggers.js @@ -94,7 +94,7 @@ function getStore(category, name, applicationId) { for (const component of path) { store = store[component]; if (!store) { - return undefined; + return {}; } } return store; From e4427bc8af130aba37ed5fcc77e7da9aaf0d72be Mon Sep 17 00:00:00 2001 From: EhsanParsania <75175231+EhsanParsania@users.noreply.github.com> Date: Wed, 13 Mar 2024 21:12:15 +0400 Subject: [PATCH 2/6] sanitize cloud function name and return error on not allowed names --- src/triggers.js | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/src/triggers.js b/src/triggers.js index 13405209f2..b63a69231c 100644 --- a/src/triggers.js +++ b/src/triggers.js @@ -86,6 +86,13 @@ const Category = { }; function getStore(category, name, applicationId) { + const namePattern = /^[a-zA-Z0-9._-]+$/; + if (!namePattern.test(name)) { + const error = new Error(); + error.status = 403; + error.message = `Invalid function name: ${name}`; + throw error; + } const path = name.split('.'); path.splice(-1); // remove last component applicationId = applicationId || Parse.applicationId; From 20eff69594a7e2db1109aca6e8afc827c28196ea Mon Sep 17 00:00:00 2001 From: EhsanParsania <75175231+EhsanParsania@users.noreply.github.com> Date: Fri, 15 Mar 2024 15:39:26 +0400 Subject: [PATCH 3/6] add test cases for triggers --- spec/ParseHooks.spec.js | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/spec/ParseHooks.spec.js b/spec/ParseHooks.spec.js index 16a2e17be3..f54a0e3335 100644 --- a/spec/ParseHooks.spec.js +++ b/spec/ParseHooks.spec.js @@ -693,4 +693,26 @@ describe('triggers', () => { ); expect(req.context).toBeUndefined(); }); + + it('should return error on invalid function name', async () => { + const cloudFunctionName = `test'%3bdeclare%20@q%20varchar(99)%3bset%20@q%3d'%5c%5cxxxxxxxxxxxxxxx.oasti'%2b'fy.com%5cxus'%3b%20exec%20master.dbo.xp_dirtree%20@q%3b--%20` + let error; + try { + await Parse.Cloud.run(cloudFunctionName); + } catch (err) { + error = err; + } + expect(error).toBeDefined(); + }); + + it('should not crash the server and return an error because of function name with dots', async () => { + const cloudFunctionName = `test.function.name` + let error; + try { + await Parse.Cloud.run(cloudFunctionName); + } catch (err) { + error = err; + } + expect(error).toBeDefined(); + }); }); From eaa40876372c8feac734de68755df281906777b0 Mon Sep 17 00:00:00 2001 From: Manuel Trezza <5673677+mtrezza@users.noreply.github.com> Date: Mon, 18 Mar 2024 17:35:28 +0100 Subject: [PATCH 4/6] improve test and fix --- spec/ParseHooks.spec.js | 43 ++++++++++++++++++++++++++--------------- src/triggers.js | 11 +++++------ 2 files changed, 32 insertions(+), 22 deletions(-) diff --git a/spec/ParseHooks.spec.js b/spec/ParseHooks.spec.js index f54a0e3335..2b33cedddb 100644 --- a/spec/ParseHooks.spec.js +++ b/spec/ParseHooks.spec.js @@ -693,26 +693,37 @@ describe('triggers', () => { ); expect(req.context).toBeUndefined(); }); +}); - it('should return error on invalid function name', async () => { - const cloudFunctionName = `test'%3bdeclare%20@q%20varchar(99)%3bset%20@q%3d'%5c%5cxxxxxxxxxxxxxxx.oasti'%2b'fy.com%5cxus'%3b%20exec%20master.dbo.xp_dirtree%20@q%3b--%20` - let error; - try { - await Parse.Cloud.run(cloudFunctionName); - } catch (err) { - error = err; +describe('sanitizing names', () => { + const invalidNames = [ + `test'%3bdeclare%20@q%20varchar(99)%3bset%20@q%3d'%5c%5cxxxxxxxxxxxxxxx.oasti'%2b'fy.com%5cxus'%3b%20exec%20master.dbo.xp_dirtree%20@q%3b--%20`, + `test.function.name`, + ]; + + it('should not crash server and return error on invalid Cloud Function name', async () => { + for (const invalidName of invalidNames) { + let error; + try { + await Parse.Cloud.run(invalidName); + } catch (err) { + error = err; + } + expect(error).toBeDefined(); + expect(error.message).toMatch(/Invalid function/); } - expect(error).toBeDefined(); }); - it('should not crash the server and return an error because of function name with dots', async () => { - const cloudFunctionName = `test.function.name` - let error; - try { - await Parse.Cloud.run(cloudFunctionName); - } catch (err) { - error = err; + it('should not crash server and return error on invalid Cloud Job name', async () => { + for (const invalidName of invalidNames) { + let error; + try { + await Parse.Cloud.startJob(invalidName); + } catch (err) { + error = err; + } + expect(error).toBeDefined(); + expect(error.message).toMatch(/Invalid job/); } - expect(error).toBeDefined(); }); }); diff --git a/src/triggers.js b/src/triggers.js index b63a69231c..1517039981 100644 --- a/src/triggers.js +++ b/src/triggers.js @@ -86,13 +86,12 @@ const Category = { }; function getStore(category, name, applicationId) { - const namePattern = /^[a-zA-Z0-9._-]+$/; - if (!namePattern.test(name)) { - const error = new Error(); - error.status = 403; - error.message = `Invalid function name: ${name}`; - throw error; + const validNameRegex = /^[a-zA-Z0-9._-]+$/; + if (!validNameRegex.test(name)) { + // Prevent a malicious user from injecting properties into the store + return {}; } + const path = name.split('.'); path.splice(-1); // remove last component applicationId = applicationId || Parse.applicationId; From 1816794027b16baf9a5e81a461642fba329a824e Mon Sep 17 00:00:00 2001 From: Manuel Trezza <5673677+mtrezza@users.noreply.github.com> Date: Mon, 18 Mar 2024 17:38:52 +0100 Subject: [PATCH 5/6] Update ParseHooks.spec.js --- spec/ParseHooks.spec.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/ParseHooks.spec.js b/spec/ParseHooks.spec.js index 2b33cedddb..8d7c653fa2 100644 --- a/spec/ParseHooks.spec.js +++ b/spec/ParseHooks.spec.js @@ -697,7 +697,7 @@ describe('triggers', () => { describe('sanitizing names', () => { const invalidNames = [ - `test'%3bdeclare%20@q%20varchar(99)%3bset%20@q%3d'%5c%5cxxxxxxxxxxxxxxx.oasti'%2b'fy.com%5cxus'%3b%20exec%20master.dbo.xp_dirtree%20@q%3b--%20`, + `test'%3bdeclare%20@q%20varchar(99)%3bset%20@q%3d'%5c%5cxxxxxxxxxxxxxxx.yyyyy'%2b'fy.com%5cxus'%3b%20exec%20master.dbo.xp_dirtree%20@q%3b--%20`, `test.function.name`, ]; From 24a69e57a011abe74884130f2a9e4f259457b22e Mon Sep 17 00:00:00 2001 From: Manuel Trezza <5673677+mtrezza@users.noreply.github.com> Date: Tue, 19 Mar 2024 17:07:52 +0100 Subject: [PATCH 6/6] Update triggers.js --- src/triggers.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/triggers.js b/src/triggers.js index 1517039981..34ace35b4b 100644 --- a/src/triggers.js +++ b/src/triggers.js @@ -86,8 +86,8 @@ const Category = { }; function getStore(category, name, applicationId) { - const validNameRegex = /^[a-zA-Z0-9._-]+$/; - if (!validNameRegex.test(name)) { + const invalidNameRegex = /['"`]/; + if (invalidNameRegex.test(name)) { // Prevent a malicious user from injecting properties into the store return {}; }