diff --git a/spec/vulnerabilities.spec.js b/spec/vulnerabilities.spec.js index a4c8903465..662438f8ba 100644 --- a/spec/vulnerabilities.spec.js +++ b/spec/vulnerabilities.spec.js @@ -7804,4 +7804,182 @@ describe('Vulnerabilities', () => { } }); }); + + describe('(GHSA-jhh9-hrgh-c9gv) Transactional batch request can roll back or block writes of other clients', () => { + const AppCache = require('../lib/cache').AppCache; + const headers = { + 'X-Parse-Application-Id': 'test', + 'X-Parse-REST-API-Key': 'rest', + 'Content-Type': 'application/json', + }; + const post = (path, body) => + request({ + method: 'POST', + url: `http://localhost:8378/1${path}`, + headers, + body, + }); + const get = path => + request({ + url: `http://localhost:8378/1${path}`, + headers, + }); + const sleep = ms => new Promise(resolve => setTimeout(resolve, ms)); + const expectSeparateDatabaseControllers = () => { + expect(Config.get('test').database).not.toBe(Config.get('test').database); + }; + const expectServerConfigInCache = () => { + const cachedConfig = AppCache.get('test'); + expect(cachedConfig.databaseController).toBeDefined(); + expect(cachedConfig.database).toBeUndefined(); + }; + + it('does not share the database controller after the master key is loaded from a function', async () => { + await reconfigureServer({ masterKey: () => 'test' }); + await post('/classes/TestObject', { key: 'value' }); + expect(Config.get('test').masterKeyCache.masterKey).toBe('test'); + expectServerConfigInCache(); + expectSeparateDatabaseControllers(); + }); + + it('does not share the database controller after the master key is reloaded', async () => { + await reconfigureServer({ masterKey: () => 'test', masterKeyTtl: 1000 }); + await post('/classes/TestObject', { key: 'value' }); + Config.get('test').masterKeyCache.expiresAt = new Date(0); + await post('/classes/TestObject', { key: 'value' }); + expect(Config.get('test').masterKeyCache.expiresAt.getTime()).toBeGreaterThan(Date.now()); + expectServerConfigInCache(); + expectSeparateDatabaseControllers(); + }); + + it('does not share the database controller after a Cloud Code rate limit is registered', async () => { + Parse.Cloud.define('rateLimitedFunction', () => 'ok', { + rateLimit: { requestTimeWindow: 10000, requestCount: 1 }, + }); + expectSeparateDatabaseControllers(); + }); + + it('does not share the database controller after the server config is set via Parse.Server', async () => { + const config = Parse.Server; + config.silent = false; + Parse.Server = config; + expectSeparateDatabaseControllers(); + }); + + it('rejects a transactional session while another one is active on the same database controller', async () => { + const database = Config.get('test').database; + spyOn(database.adapter, 'createTransactionalSession').and.resolveTo({}); + await database.createTransactionalSession(); + await expectAsync(database.createTransactionalSession()).toBeRejectedWithError( + 'There is already an active transactional session' + ); + expect(database.adapter.createTransactionalSession).toHaveBeenCalledTimes(1); + }); + + it('rejects concurrent transactional sessions on the same database controller', async () => { + const database = Config.get('test').database; + spyOn(database.adapter, 'createTransactionalSession').and.resolveTo({}); + const results = await Promise.allSettled([ + database.createTransactionalSession(), + database.createTransactionalSession(), + ]); + expect(results.map(result => result.status)).toEqual(['fulfilled', 'rejected']); + expect(results[1].reason.message).toBe('There is already an active transactional session'); + expect(database.adapter.createTransactionalSession).toHaveBeenCalledTimes(1); + }); + + it('allows a transactional session after the creation of a transactional session failed', async () => { + const database = Config.get('test').database; + spyOn(database.adapter, 'createTransactionalSession').and.returnValues( + Promise.reject(new Error('creation failed')), + Promise.resolve({}) + ); + await expectAsync(database.createTransactionalSession()).toBeRejectedWithError( + 'creation failed' + ); + await expectAsync(database.createTransactionalSession()).toBeResolved(); + }); + + it('allows a transactional session after the creation of a transactional session threw', async () => { + const database = Config.get('test').database; + let calls = 0; + spyOn(database.adapter, 'createTransactionalSession').and.callFake(() => { + if (++calls === 1) { + throw new Error('creation threw'); + } + return Promise.resolve({}); + }); + await expectAsync(database.createTransactionalSession()).toBeRejectedWithError( + 'creation threw' + ); + await expectAsync(database.createTransactionalSession()).toBeResolved(); + }); + + if ( + ['replicaset', 'replset'].includes(process.env.MONGODB_TOPOLOGY) || + process.env.PARSE_SERVER_TEST_DB === 'postgres' + ) { + describe('transactions', () => { + beforeEach(async () => { + await reconfigureServer({ masterKey: () => 'test' }); + // Transactions only work on existing classes + for (const className of ['SlowObject', 'FailingObject', 'OtherObject']) { + await post(`/classes/${className}`, { key: 'value' }); + } + Parse.Cloud.beforeSave('SlowObject', () => sleep(500)); + }); + + it('keeps a write of another client that runs during a failing transactional batch', async () => { + const batch = expectAsync( + post('/batch', { + transaction: true, + requests: [ + { method: 'POST', path: '/1/classes/SlowObject', body: { key: 'value' } }, + { method: 'POST', path: '/1/classes/FailingObject', body: { key: 10 } }, + ], + }) + ).toBeRejected(); + await sleep(150); + const response = await post('/classes/OtherObject', { key: 'other client' }); + await batch; + const object = await get(`/classes/OtherObject/${response.data.objectId}`); + expect(object.data.key).toBe('other client'); + }); + + it('completes concurrent transactional batches', async () => { + const batch = () => + post('/batch', { + transaction: true, + requests: [ + { method: 'POST', path: '/1/classes/SlowObject', body: { key: 'value' } }, + { method: 'POST', path: '/1/classes/OtherObject', body: { key: 'value' } }, + ], + }); + const first = batch(); + await sleep(100); + const responses = await Promise.all([first, batch()]); + for (const response of responses) { + expect(response.data.length).toBe(2); + expect(response.data.every(result => result.success)).toBeTrue(); + } + const objects = await get('/classes/OtherObject'); + expect(objects.data.results.length).toBe(3); + }); + + it('does not fail writes of other clients after a failing transactional batch', async () => { + await expectAsync( + post('/batch', { + transaction: true, + requests: [ + { method: 'POST', path: '/1/classes/OtherObject', body: { key: 'value' } }, + { method: 'POST', path: '/1/classes/FailingObject', body: { key: 10 } }, + ], + }) + ).toBeRejected(); + const response = await post('/classes/OtherObject', { key: 'other client' }); + expect(response.data.objectId).toBeDefined(); + }); + }); + } + }); }); diff --git a/src/Config.js b/src/Config.js index 95543c6c6b..f562e1da24 100644 --- a/src/Config.js +++ b/src/Config.js @@ -52,12 +52,17 @@ export class Config { const config = new Config(); config.applicationId = applicationId; Object.keys(cacheInfo).forEach(key => { - if (key == 'databaseController') { - config.database = new DatabaseController(cacheInfo.databaseController.adapter, config); - } else { + if (key != 'databaseController' && key != 'database') { config[key] = cacheInfo[key]; } }); + // Always create a new database controller, as it holds request-scoped state such as the + // transactional session; a request-scoped config in the cache has `database` instead of + // `databaseController` + const databaseController = cacheInfo.databaseController || cacheInfo.database; + if (databaseController) { + config.database = new DatabaseController(databaseController.adapter, config); + } config.mount = removeTrailingSlash(mount); config.generateSessionExpiresAt = config.generateSessionExpiresAt.bind(config); config.generateEmailVerifyTokenExpiresAt = config.generateEmailVerifyTokenExpiresAt.bind( @@ -989,7 +994,11 @@ export class Config { const expiresAt = this.masterKeyTtl ? new Date(Date.now() + 1000 * this.masterKeyTtl) : null this.masterKeyCache = { masterKey, expiresAt }; - Config.put(this); + // Update only the cached server config, as this config is request-scoped + const serverConfig = AppCache.get(this.applicationId); + if (serverConfig) { + serverConfig.masterKeyCache = this.masterKeyCache; + } return this.masterKeyCache.masterKey; } diff --git a/src/Controllers/DatabaseController.js b/src/Controllers/DatabaseController.js index d598cacac5..9fdf6232bc 100644 --- a/src/Controllers/DatabaseController.js +++ b/src/Controllers/DatabaseController.js @@ -451,6 +451,7 @@ class DatabaseController { // multiple schemas, so instead use loadSchema to get a schema. this.schemaPromise = null; this._transactionalSession = null; + this._transactionalSessionPending = false; this.options = options; } @@ -1925,9 +1926,20 @@ class DatabaseController { } createTransactionalSession() { - return this.adapter.createTransactionalSession().then(transactionalSession => { - this._transactionalSession = transactionalSession; - }); + if (this._transactionalSession || this._transactionalSessionPending) { + return Promise.reject(new Error('There is already an active transactional session')); + } + // Reserve the session before it is created, without setting `_transactionalSession`, which + // concurrent writes on this controller would otherwise use + this._transactionalSessionPending = true; + return Promise.resolve() + .then(() => this.adapter.createTransactionalSession()) + .then(transactionalSession => { + this._transactionalSession = transactionalSession; + }) + .finally(() => { + this._transactionalSessionPending = false; + }); } commitTransactionalSession() {