Skip to content

Commit 552c6dd

Browse files
mtrezzamtrezza
authored
fix: Server option routeAllowList is bypassable through batch sub-requests ([GHSA-p84r-h6rx-f2xr](GHSA-p84r-h6rx-f2xr)) (parse-community#10482)
1 parent 0ae0ed3 commit 552c6dd

3 files changed

Lines changed: 154 additions & 22 deletions

File tree

‎spec/RouteAllowList.spec.js‎

Lines changed: 107 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -375,6 +375,113 @@ describe('routeAllowList', () => {
375375
});
376376
});
377377

378+
describe('batch sub-requests', () => {
379+
// routeAllowList must be enforced per batch sub-request. The outer
380+
// enforceRouteAllowList middleware runs only on the outer /batch URL,
381+
// so without per-sub-request enforcement an operator who allowlists
382+
// `batch` would accidentally expose every REST route reachable through
383+
// batch sub-request dispatch.
384+
const restRequest = require('../lib/request');
385+
const headers = {
386+
'Content-Type': 'application/json',
387+
'X-Parse-Application-Id': 'test',
388+
'X-Parse-REST-API-Key': 'rest',
389+
};
390+
391+
it('blocks a batch GET sub-request whose path is not allowlisted', async () => {
392+
await reconfigureServer({ routeAllowList: ['batch'] });
393+
await new Parse.Object('Blocked').save({ secret: 'x' }, { useMasterKey: true });
394+
try {
395+
await restRequest({
396+
method: 'POST',
397+
headers,
398+
url: 'http://localhost:8378/1/batch',
399+
body: JSON.stringify({
400+
requests: [{ method: 'GET', path: '/1/classes/Blocked' }],
401+
}),
402+
});
403+
fail('batch sub-request to a blocked route should have been rejected');
404+
} catch (e) {
405+
expect(e.data.code).toBe(Parse.Error.OPERATION_FORBIDDEN);
406+
}
407+
});
408+
409+
it('blocks a batch POST sub-request whose path is not allowlisted', async () => {
410+
await reconfigureServer({ routeAllowList: ['batch'] });
411+
try {
412+
await restRequest({
413+
method: 'POST',
414+
headers,
415+
url: 'http://localhost:8378/1/batch',
416+
body: JSON.stringify({
417+
requests: [{ method: 'POST', path: '/1/classes/Blocked', body: { x: 1 } }],
418+
}),
419+
});
420+
fail('batch sub-request POST to a blocked route should have been rejected');
421+
} catch (e) {
422+
expect(e.data.code).toBe(Parse.Error.OPERATION_FORBIDDEN);
423+
}
424+
const query = new Parse.Query('Blocked');
425+
const results = await query.find({ useMasterKey: true });
426+
expect(results.length).toBe(0);
427+
});
428+
429+
it('allows a batch sub-request whose path matches the allow list', async () => {
430+
await reconfigureServer({ routeAllowList: ['batch', 'classes/Allowed'] });
431+
const response = await restRequest({
432+
method: 'POST',
433+
headers,
434+
url: 'http://localhost:8378/1/batch',
435+
body: JSON.stringify({
436+
requests: [{ method: 'POST', path: '/1/classes/Allowed', body: { x: 1 } }],
437+
}),
438+
});
439+
expect(response.data.length).toBe(1);
440+
expect(response.data[0].success.objectId).toBeDefined();
441+
});
442+
443+
it('rejects the entire batch if any sub-request is not allowlisted', async () => {
444+
await reconfigureServer({ routeAllowList: ['batch', 'classes/Allowed'] });
445+
try {
446+
await restRequest({
447+
method: 'POST',
448+
headers,
449+
url: 'http://localhost:8378/1/batch',
450+
body: JSON.stringify({
451+
requests: [
452+
{ method: 'POST', path: '/1/classes/Allowed', body: { x: 1 } },
453+
{ method: 'POST', path: '/1/classes/Blocked', body: { y: 2 } },
454+
],
455+
}),
456+
});
457+
fail('batch with any disallowed sub-request should have been rejected');
458+
} catch (e) {
459+
expect(e.data.code).toBe(Parse.Error.OPERATION_FORBIDDEN);
460+
}
461+
const allowedQuery = new Parse.Query('Allowed');
462+
const allowedResults = await allowedQuery.find({ useMasterKey: true });
463+
expect(allowedResults.length).toBe(0);
464+
});
465+
466+
it('allows master key to bypass sub-request allow-list check', async () => {
467+
await reconfigureServer({ routeAllowList: ['batch'] });
468+
const response = await restRequest({
469+
method: 'POST',
470+
headers: {
471+
'Content-Type': 'application/json',
472+
'X-Parse-Application-Id': 'test',
473+
'X-Parse-Master-Key': 'test',
474+
},
475+
url: 'http://localhost:8378/1/batch',
476+
body: JSON.stringify({
477+
requests: [{ method: 'POST', path: '/1/classes/Blocked', body: { x: 1 } }],
478+
}),
479+
});
480+
expect(response.data.length).toBe(1);
481+
expect(response.data[0].success.objectId).toBeDefined();
482+
});
483+
});
484+
378485
it_id('229cab22-dad3-4d08-8de5-64d813658596')(it)('should block all route groups when not in allow list', async () => {
379486
await reconfigureServer({
380487
routeAllowList: ['classes/GameScore'],

‎src/batch.js‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,7 @@
11
const Parse = require('parse/node').Parse;
22
const path = require('path');
3+
const { isRouteAllowed } = require('./middlewares');
4+
const { createSanitizedError } = require('./Error');
35
// These methods handle batch requests.
46
const batchPath = '/batch';
57

@@ -104,6 +106,17 @@ async function handleBatch(router, req) {
104106
if ((restRequest.method || 'GET').toUpperCase() === 'POST' && routablePath === batchPath) {
105107
throw new Parse.Error(Parse.Error.INVALID_JSON, 'nested batch requests are not allowed');
106108
}
109+
// Re-enforce routeAllowList on each sub-request. The enforceRouteAllowList
110+
// middleware runs once on the outer /batch URL, so without this check an
111+
// operator who allowlists `batch` would expose every route reachable via
112+
// sub-request dispatch.
113+
if (!isRouteAllowed(routablePath, req.config, req.auth)) {
114+
throw createSanitizedError(
115+
Parse.Error.OPERATION_FORBIDDEN,
116+
`Route not allowed by routeAllowList: ${(restRequest.method || 'GET').toUpperCase()} ${routablePath}`,
117+
req.config
118+
);
119+
}
107120
for (const limit of rateLimits) {
108121
const pathExp = limit.path.regexp || limit.path;
109122
if (!pathExp.test(routablePath)) {

‎src/middlewares.js‎

Lines changed: 34 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -519,41 +519,53 @@ export function handleParseHealth(options) {
519519
};
520520
}
521521

522-
export function enforceRouteAllowList(req, res, next) {
523-
const config = req.config;
524-
if (!config || config.routeAllowList === undefined || config.routeAllowList === null) {
525-
return next();
526-
}
527-
if (req.auth && (req.auth.isMaster || req.auth.isMaintenance)) {
528-
return next();
529-
}
530-
let path = req.originalUrl;
531-
if (config.mount) {
532-
const mountPath = new URL(config.mount).pathname;
533-
if (path.startsWith(mountPath)) {
534-
path = path.substring(mountPath.length);
522+
function normalizeRouteAllowListPath(path, mount) {
523+
let normalized = path;
524+
if (mount) {
525+
const mountPath = new URL(mount).pathname;
526+
if (normalized.startsWith(mountPath)) {
527+
normalized = normalized.substring(mountPath.length);
535528
}
536529
}
537-
if (path.startsWith('/')) {
538-
path = path.substring(1);
530+
if (normalized.startsWith('/')) {
531+
normalized = normalized.substring(1);
539532
}
540-
if (path.endsWith('/')) {
541-
path = path.substring(0, path.length - 1);
533+
if (normalized.endsWith('/')) {
534+
normalized = normalized.substring(0, normalized.length - 1);
542535
}
543-
const queryIndex = path.indexOf('?');
536+
const queryIndex = normalized.indexOf('?');
544537
if (queryIndex !== -1) {
545-
path = path.substring(0, queryIndex);
538+
normalized = normalized.substring(0, queryIndex);
546539
}
540+
return normalized;
541+
}
542+
543+
export function isRouteAllowed(path, config, auth) {
544+
if (!config || config.routeAllowList === undefined || config.routeAllowList === null) {
545+
return true;
546+
}
547+
if (auth && (auth.isMaster || auth.isMaintenance)) {
548+
return true;
549+
}
550+
const normalized = normalizeRouteAllowListPath(path, config.mount);
547551
const regexes = config._routeAllowListRegex || [];
548552
for (const regex of regexes) {
549-
if (regex.test(path)) {
550-
return next();
553+
if (regex.test(normalized)) {
554+
return true;
551555
}
552556
}
557+
return false;
558+
}
559+
560+
export function enforceRouteAllowList(req, res, next) {
561+
if (isRouteAllowed(req.originalUrl, req.config, req.auth)) {
562+
return next();
563+
}
564+
const path = normalizeRouteAllowListPath(req.originalUrl, req.config?.mount);
553565
throw createSanitizedError(
554566
Parse.Error.OPERATION_FORBIDDEN,
555567
`Route not allowed by routeAllowList: ${req.method} ${path}`,
556-
config
568+
req.config
557569
);
558570
}
559571

0 commit comments

Comments
 (0)