From 48e9f840ddc5646d295479c8790bf7c86fcf3812 Mon Sep 17 00:00:00 2001 From: Kunal Gangakhedkar Date: Fri, 5 Feb 2016 01:21:58 +0530 Subject: [PATCH 1/3] fix for #246. If the incoming request has app id specified both in the header AND the JSON body, then check that both are the same. Flag the request as invalid if they are not the same. If the app id is not specified in the headers, then pick it up from the JSON body. Moreover, remove the app config keys (app id, master key, js key, .net key etc.) from the request body unconditionally - ie. even if those are specified in the header. This is so that if the proxy fronting the app is not able these keys from the body. Signed-off-by: Kunal Gangakhedkar --- middlewares.js | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/middlewares.js b/middlewares.js index bb2512391a..9c9be74cc5 100644 --- a/middlewares.js +++ b/middlewares.js @@ -28,7 +28,6 @@ function handleParseHeaders(req, res, next) { var fileViaJSON = false; - if (!info.appId || !cache.apps[info.appId]) { // See if we can find the app id on the body. if (req.body instanceof Buffer) { // The only chance to find the app id is if this is a file @@ -44,7 +43,11 @@ function handleParseHeaders(req, res, next) { || cache.apps[req.body._ApplicationId]['masterKey'] === info.masterKey) ) { - info.appId = req.body._ApplicationId; + if ((info.appId) && (info.appId !== req.body._ApplicationId)) + return invalidRequest(req, res); + else if (!info.appId) + info.appId = req.body._ApplicationId; + info.javascriptKey = req.body._JavaScriptKey || ''; delete req.body._ApplicationId; delete req.body._JavaScriptKey; @@ -69,7 +72,6 @@ function handleParseHeaders(req, res, next) { } else { return invalidRequest(req, res); } - } if (fileViaJSON) { // We need to repopulate req.body with a buffer From a98652c5d1bae6a1b08a4871197ab432589ac4e4 Mon Sep 17 00:00:00 2001 From: Kunal Gangakhedkar Date: Mon, 8 Feb 2016 23:29:03 +0530 Subject: [PATCH 2/3] indentation fixes. De-indent the block after removing the if conditional for appid passed in the header. Signed-off-by: Kunal Gangakhedkar --- middlewares.js | 82 +++++++++++++++++++++++++------------------------- 1 file changed, 41 insertions(+), 41 deletions(-) diff --git a/middlewares.js b/middlewares.js index 9c9be74cc5..c404902e2e 100644 --- a/middlewares.js +++ b/middlewares.js @@ -28,50 +28,50 @@ function handleParseHeaders(req, res, next) { var fileViaJSON = false; - // See if we can find the app id on the body. - 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); - fileViaJSON = true; - } + // See if we can find the app id on the body. + 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); + fileViaJSON = true; + } - if (req.body && req.body._ApplicationId - && cache.apps[req.body._ApplicationId] - && ( - !info.masterKey - || - cache.apps[req.body._ApplicationId]['masterKey'] === info.masterKey) - ) { - if ((info.appId) && (info.appId !== req.body._ApplicationId)) - return invalidRequest(req, res); - else if (!info.appId) - info.appId = req.body._ApplicationId; - - info.javascriptKey = req.body._JavaScriptKey || ''; - delete req.body._ApplicationId; - delete req.body._JavaScriptKey; - // TODO: test that the REST API formats generated by the other - // SDKs are handled ok - if (req.body._ClientVersion) { - info.clientVersion = req.body._ClientVersion; - delete req.body._ClientVersion; - } - if (req.body._InstallationId) { - info.installationId = req.body._InstallationId; - delete req.body._InstallationId; - } - if (req.body._SessionToken) { - info.sessionToken = req.body._SessionToken; - delete req.body._SessionToken; - } - if (req.body._MasterKey) { - info.masterKey = req.body._MasterKey; - delete req.body._MasterKey; - } - } else { + if (req.body && req.body._ApplicationId + && cache.apps[req.body._ApplicationId] + && ( + !info.masterKey + || + cache.apps[req.body._ApplicationId]['masterKey'] === info.masterKey) + ) { + if ((info.appId) && (info.appId !== req.body._ApplicationId)) return invalidRequest(req, res); + else if (!info.appId) + info.appId = req.body._ApplicationId; + + info.javascriptKey = req.body._JavaScriptKey || ''; + delete req.body._ApplicationId; + delete req.body._JavaScriptKey; + // TODO: test that the REST API formats generated by the other + // SDKs are handled ok + if (req.body._ClientVersion) { + info.clientVersion = req.body._ClientVersion; + delete req.body._ClientVersion; } + if (req.body._InstallationId) { + info.installationId = req.body._InstallationId; + delete req.body._InstallationId; + } + if (req.body._SessionToken) { + info.sessionToken = req.body._SessionToken; + delete req.body._SessionToken; + } + if (req.body._MasterKey) { + info.masterKey = req.body._MasterKey; + delete req.body._MasterKey; + } + } else { + return invalidRequest(req, res); + } if (fileViaJSON) { // We need to repopulate req.body with a buffer From 1dcb1e63a35d6ae7cbe9228a5289d134ef59826f Mon Sep 17 00:00:00 2001 From: Kunal Gangakhedkar Date: Mon, 8 Feb 2016 23:52:45 +0530 Subject: [PATCH 3/3] apply the same checks for other hidden attributes. Signed-off-by: Kunal Gangakhedkar --- middlewares.js | 41 +++++++++++++++++++++++++++++++---------- 1 file changed, 31 insertions(+), 10 deletions(-) diff --git a/middlewares.js b/middlewares.js index c404902e2e..f73034900a 100644 --- a/middlewares.js +++ b/middlewares.js @@ -48,27 +48,48 @@ function handleParseHeaders(req, res, next) { else if (!info.appId) info.appId = req.body._ApplicationId; - info.javascriptKey = req.body._JavaScriptKey || ''; delete req.body._ApplicationId; + + if ((info.javascriptKey) && (info.javascriptKey !== req.body._JavaScriptKey)) + return invalidRequest(req, res); + else if (!info.javascriptKey) + info.javascriptKey = req.body._JavaScriptKey || ''; + delete req.body._JavaScriptKey; - // TODO: test that the REST API formats generated by the other - // SDKs are handled ok - if (req.body._ClientVersion) { - info.clientVersion = req.body._ClientVersion; - delete req.body._ClientVersion; - } + if (req.body._InstallationId) { - info.installationId = req.body._InstallationId; + if ((info.installationId) && (info.installationId !== req.body._InstallationId)) + return invalidRequest(req, res); + else if (!info.installationId) + info.installationId = req.body._InstallationId; + delete req.body._InstallationId; } + if (req.body._SessionToken) { - info.sessionToken = req.body._SessionToken; + if ((info.sessionToken) && (info.sessionToken !== req.body._SessionToken)) + return invalidRequest(req, res); + else if (!info.sessionToken) + info.sessionToken = req.body._SessionToken; + delete req.body._SessionToken; } + if (req.body._MasterKey) { - info.masterKey = req.body._MasterKey; + if ((info.masterKey) && (info.masterKey !== req.body._MasterKey)) + return invalidRequest(req, res); + else if (!info.masterKey) + info.masterKey = req.body._MasterKey; + delete req.body._MasterKey; } + + // TODO: test that the REST API formats generated by the other + // SDKs are handled ok + if (req.body._ClientVersion) { + info.clientVersion = req.body._ClientVersion; + delete req.body._ClientVersion; + } } else { return invalidRequest(req, res); }