From b8685ac320638aea791d4c1e681d6280e4bdf887 Mon Sep 17 00:00:00 2001 From: Gordon Sun Date: Tue, 14 Apr 2020 22:35:16 -0700 Subject: [PATCH] catch JSON.parse and return 403 properly --- spec/Middlewares.spec.js | 47 +++++++++++++++++++++++++--------------- src/middlewares.js | 19 +++++++++++----- 2 files changed, 43 insertions(+), 23 deletions(-) diff --git a/spec/Middlewares.spec.js b/spec/Middlewares.spec.js index c81bd1b9fe..1c48ab675c 100644 --- a/spec/Middlewares.spec.js +++ b/spec/Middlewares.spec.js @@ -12,7 +12,7 @@ describe('middlewares', () => { _ApplicationId: 'FakeAppId', }, headers: {}, - get: key => { + get: (key) => { return fakeReq.headers[key.toLowerCase()]; }, }; @@ -24,7 +24,7 @@ describe('middlewares', () => { AppCache.del(fakeReq.body._ApplicationId); }); - it('should use _ContentType if provided', done => { + it('should use _ContentType if provided', (done) => { expect(fakeReq.headers['content-type']).toEqual(undefined); const contentType = 'image/jpeg'; fakeReq.body._ContentType = contentType; @@ -64,7 +64,7 @@ describe('middlewares', () => { expect(fakeRes.status).toHaveBeenCalledWith(403); }); - it('should succeed when any one of the configured keys supplied', done => { + it('should succeed when any one of the configured keys supplied', (done) => { AppCache.put(fakeReq.body._ApplicationId, { clientKey: 'clientKey', masterKey: 'masterKey', @@ -77,7 +77,7 @@ describe('middlewares', () => { }); }); - it('should succeed when client key supplied but empty', done => { + it('should succeed when client key supplied but empty', (done) => { AppCache.put(fakeReq.body._ApplicationId, { clientKey: '', masterKey: 'masterKey', @@ -90,7 +90,7 @@ describe('middlewares', () => { }); }); - it('should succeed when no keys are configured and none supplied', done => { + it('should succeed when no keys are configured and none supplied', (done) => { AppCache.put(fakeReq.body._ApplicationId, { masterKey: 'masterKey', }); @@ -110,22 +110,22 @@ describe('middlewares', () => { const BodyKeys = Object.keys(BodyParams); - BodyKeys.forEach(infoKey => { + BodyKeys.forEach((infoKey) => { const bodyKey = BodyParams[infoKey]; const keyValue = 'Fake' + bodyKey; // javascriptKey is the only one that gets defaulted, const otherKeys = BodyKeys.filter( - otherKey => otherKey !== infoKey && otherKey !== 'javascriptKey' + (otherKey) => otherKey !== infoKey && otherKey !== 'javascriptKey' ); - it(`it should pull ${bodyKey} into req.info`, done => { + it(`it should pull ${bodyKey} into req.info`, (done) => { fakeReq.body[bodyKey] = keyValue; middlewares.handleParseHeaders(fakeReq, fakeRes, () => { expect(fakeReq.body[bodyKey]).toEqual(undefined); expect(fakeReq.info[infoKey]).toEqual(keyValue); - otherKeys.forEach(otherKey => { + otherKeys.forEach((otherKey) => { expect(fakeReq.info[otherKey]).toEqual(undefined); }); @@ -145,7 +145,7 @@ describe('middlewares', () => { expect(fakeRes.status).toHaveBeenCalledWith(403); }); - it('should succeed if the ip does belong to masterKeyIps list', done => { + it('should succeed if the ip does belong to masterKeyIps list', (done) => { AppCache.put(fakeReq.body._ApplicationId, { masterKey: 'masterKey', masterKeyIps: ['ip1', 'ip2'], @@ -169,7 +169,7 @@ describe('middlewares', () => { expect(fakeRes.status).toHaveBeenCalledWith(403); }); - it('should succeed if the connection.remoteAddress does belong to masterKeyIps list', done => { + it('should succeed if the connection.remoteAddress does belong to masterKeyIps list', (done) => { AppCache.put(fakeReq.body._ApplicationId, { masterKey: 'masterKey', masterKeyIps: ['ip1', 'ip2'], @@ -193,7 +193,7 @@ describe('middlewares', () => { expect(fakeRes.status).toHaveBeenCalledWith(403); }); - it('should succeed if the socket.remoteAddress does belong to masterKeyIps list', done => { + it('should succeed if the socket.remoteAddress does belong to masterKeyIps list', (done) => { AppCache.put(fakeReq.body._ApplicationId, { masterKey: 'masterKey', masterKeyIps: ['ip1', 'ip2'], @@ -217,7 +217,7 @@ describe('middlewares', () => { expect(fakeRes.status).toHaveBeenCalledWith(403); }); - it('should succeed if the connection.socket.remoteAddress does belong to masterKeyIps list', done => { + it('should succeed if the connection.socket.remoteAddress does belong to masterKeyIps list', (done) => { AppCache.put(fakeReq.body._ApplicationId, { masterKey: 'masterKey', masterKeyIps: ['ip1', 'ip2'], @@ -230,7 +230,7 @@ describe('middlewares', () => { }); }); - it('should allow any ip to use masterKey if masterKeyIps is empty', done => { + it('should allow any ip to use masterKey if masterKeyIps is empty', (done) => { AppCache.put(fakeReq.body._ApplicationId, { masterKey: 'masterKey', masterKeyIps: [], @@ -243,7 +243,7 @@ describe('middlewares', () => { }); }); - it('should succeed if xff header does belong to masterKeyIps', done => { + it('should succeed if xff header does belong to masterKeyIps', (done) => { AppCache.put(fakeReq.body._ApplicationId, { masterKey: 'masterKey', masterKeyIps: ['ip1'], @@ -256,7 +256,7 @@ describe('middlewares', () => { }); }); - it('should succeed if xff header with one ip does belong to masterKeyIps', done => { + it('should succeed if xff header with one ip does belong to masterKeyIps', (done) => { AppCache.put(fakeReq.body._ApplicationId, { masterKey: 'masterKey', masterKeyIps: ['ip1'], @@ -357,7 +357,7 @@ describe('middlewares', () => { ); }); - it('should use user provided on field userFromJWT', done => { + it('should use user provided on field userFromJWT', (done) => { AppCache.put(fakeReq.body._ApplicationId, { masterKey: 'masterKey', }); @@ -367,4 +367,17 @@ describe('middlewares', () => { done(); }); }); + + it('should give invalid response when upload file without x-parse-application-id in header', () => { + AppCache.put(fakeReq.body._ApplicationId, { + masterKey: 'masterKey', + }); + fakeReq.body = Buffer.from('fake-file'); + console.log('fakeReq.body.constructor'); + console.log(fakeReq.body.constructor); + console.log('fakeReq.body instanceof Buffer'); + console.log(fakeReq.body instanceof Buffer); + middlewares.handleParseHeaders(fakeReq, fakeRes); + expect(fakeRes.status).toHaveBeenCalledWith(403); + }); }); diff --git a/src/middlewares.js b/src/middlewares.js index 6f836119dc..681053e8e9 100644 --- a/src/middlewares.js +++ b/src/middlewares.js @@ -8,7 +8,7 @@ import defaultLogger from './logger'; export const DEFAULT_ALLOWED_HEADERS = 'X-Parse-Master-Key, X-Parse-REST-API-Key, X-Parse-Javascript-Key, X-Parse-Application-Id, X-Parse-Client-Version, X-Parse-Session-Token, X-Requested-With, X-Parse-Revocable-Session, Content-Type, Pragma, Cache-Control'; -const getMountForRequest = function(req) { +const getMountForRequest = function (req) { const mountPathLength = req.originalUrl.length - req.url.length; const mountPath = req.originalUrl.slice(0, mountPathLength); return req.protocol + '://' + req.get('host') + mountPath; @@ -59,7 +59,14 @@ export function handleParseHeaders(req, res, next) { if (req.body instanceof Buffer) { // The only chance to find the app id is if this is a file // upload that actually is a JSON body. So try to parse it. - req.body = JSON.parse(req.body); + // https://github.com/parse-community/parse-server/issues/6589 + // It is also possible that the client is trying to upload a file but forgot + // to provide x-parse-app-id in header and parse a binary file will fail + try { + req.body = JSON.parse(req.body); + } catch (e) { + return invalidRequest(req, res); + } fileViaJSON = true; } @@ -168,10 +175,10 @@ export function handleParseHeaders(req, res, next) { // Client keys are not required in parse-server, but if any have been configured in the server, validate them // to preserve original behavior. const keys = ['clientKey', 'javascriptKey', 'dotNetKey', 'restAPIKey']; - const oneKeyConfigured = keys.some(function(key) { + const oneKeyConfigured = keys.some(function (key) { return req.config[key] !== undefined; }); - const oneKeyMatches = keys.some(function(key) { + const oneKeyMatches = keys.some(function (key) { return req.config[key] !== undefined && info[key] === req.config[key]; }); @@ -225,13 +232,13 @@ export function handleParseHeaders(req, res, next) { }); } }) - .then(auth => { + .then((auth) => { if (auth) { req.auth = auth; next(); } }) - .catch(error => { + .catch((error) => { if (error instanceof Parse.Error) { next(error); return;