From 96134fa3393505628fb59144d1d44bb42f1f1b14 Mon Sep 17 00:00:00 2001 From: Nik Frunza Date: Mon, 9 Dec 2019 23:35:37 -0500 Subject: [PATCH] BE-719 Fix parameter tampering (#68) * BE-719 Fix parameter tampering Signed-off-by: nfrunza * BE-719 Exclude warning * added js/superfluous-trailing-arguments to lgtm.yml config Signed-off-by: nfrunza * BE-719 Fix parameter tampering * added tests, removed console.debug statements Signed-off-by: nfrunza * BE-719 Fix parameter tampering * removed console.debug staement Signed-off-by: nfrunza --- app/rest/dbroutes.js | 4 +-- app/rest/requestutils.js | 31 ++++++++++++++++------- app/test/fixtures/reqMultiOrgs.json | 5 ++++ app/test/fixtures/reqNoOrgs.json | 4 +++ app/test/fixtures/reqOneOrg.json | 5 ++++ app/test/requestutils.js | 38 +++++++++++++++++++++++++++++ lgtm.yml | 2 ++ package-lock.json | 23 +++++++++++++++-- package.json | 1 + 9 files changed, 100 insertions(+), 13 deletions(-) create mode 100644 app/test/fixtures/reqMultiOrgs.json create mode 100644 app/test/fixtures/reqNoOrgs.json create mode 100644 app/test/fixtures/reqOneOrg.json create mode 100644 app/test/requestutils.js diff --git a/app/rest/dbroutes.js b/app/rest/dbroutes.js index 20d6cc653..a3317efc5 100644 --- a/app/rest/dbroutes.js +++ b/app/rest/dbroutes.js @@ -118,7 +118,7 @@ const dbroutes = (router, platform) => { const channel_genesis_hash = req.params.channel_genesis_hash; const blockNum = parseInt(req.params.blocknum); let txid = parseInt(req.params.txid); - const orgs = requtil.orgsArrayToString(req.query.orgs); + const orgs = requtil.orgsArrayToString(req.query); const { from, to } = requtil.queryDatevalidator( req.query.from, req.query.to @@ -207,7 +207,7 @@ const dbroutes = (router, platform) => { async (req, res) => { const channel_genesis_hash = req.params.channel_genesis_hash; const blockNum = parseInt(req.params.blocknum); - const orgs = requtil.orgsArrayToString(req.query.orgs); + const orgs = requtil.orgsArrayToString(req.query); const { from, to } = requtil.queryDatevalidator( req.query.from, req.query.to diff --git a/app/rest/requestutils.js b/app/rest/requestutils.js index da6d3f971..93bf165e5 100644 --- a/app/rest/requestutils.js +++ b/app/rest/requestutils.js @@ -2,6 +2,7 @@ * SPDX-License-Identifier: Apache-2.0 */ +const queryString = require('query-string'); /** * * @@ -88,18 +89,30 @@ function reqPayload(req) { return requestPayload; } -const orgsArrayToString = function(orgs) { +const orgsArrayToString = function(reqQuery) { let temp = ''; - if (Array.isArray(orgs) || typeof orgs === 'object') { - orgs.forEach((element, i) => { - temp += `'${element}'`; - if (orgs.length - 1 !== i) { - temp += ','; + if (reqQuery) { + // eslint-disable-next-line spellcheck/spell-checker + // workaround 'Type confusion through parameter tampering', see `https //lgtm dot com/rules/1506301137371 ` + const orgsStr = queryString.stringify(reqQuery); + + if (orgsStr) { + const parsedReq = queryString.parse(orgsStr); + if (parsedReq && parsedReq.orgs) { + const orgsArray = parsedReq.orgs.toString().split(','); + // format DB value for IN clause, ex: in ('a', 'b', 'c') + if (orgsArray) { + orgsArray.forEach((element, i) => { + temp += `'${element}'`; + if (orgsArray.length - 1 !== i) { + temp += ','; + } + }); + } } - }); - } else if (orgs) { - temp = `'${orgs}'`; + } } + return temp; }; diff --git a/app/test/fixtures/reqMultiOrgs.json b/app/test/fixtures/reqMultiOrgs.json new file mode 100644 index 000000000..0b18471b0 --- /dev/null +++ b/app/test/fixtures/reqMultiOrgs.json @@ -0,0 +1,5 @@ +{ + "from": "Sun Dec 08 2019 22:22:00 GMT-0500 (Eastern Standard Time)", + "to": "Mon Dec 09 2019 22:22:00 GMT-0500 (Eastern Standard Time)", + "orgs": ["OrdererMSP", "Org2MSP"] +} diff --git a/app/test/fixtures/reqNoOrgs.json b/app/test/fixtures/reqNoOrgs.json new file mode 100644 index 000000000..ee46bb4a3 --- /dev/null +++ b/app/test/fixtures/reqNoOrgs.json @@ -0,0 +1,4 @@ +{ + "from": "Sun Dec 08 2019 22:22:00 GMT-0500 (Eastern Standard Time)", + "to": "Mon Dec 09 2019 22:22:00 GMT-0500 (Eastern Standard Time)" +} diff --git a/app/test/fixtures/reqOneOrg.json b/app/test/fixtures/reqOneOrg.json new file mode 100644 index 000000000..e0fe66c68 --- /dev/null +++ b/app/test/fixtures/reqOneOrg.json @@ -0,0 +1,5 @@ +{ + "from": "Sun Dec 08 2019 22:22:00 GMT-0500 (Eastern Standard Time)", + "to": "Mon Dec 09 2019 22:22:00 GMT-0500 (Eastern Standard Time)", + "orgs": "OrgOne" +} diff --git a/app/test/requestutils.js b/app/test/requestutils.js new file mode 100644 index 000000000..0f1659c5b --- /dev/null +++ b/app/test/requestutils.js @@ -0,0 +1,38 @@ +/* + SPDX-License-Identifier: Apache-2.0 +*/ + +const expect = require('chai').expect; +const assert = require('assert'); +const chai = require('chai'); +const chaiHttp = require('chai-http'); +const helper = require('../common/helper'); +const reqMultiOrgs = require('./fixtures/reqMultiOrgs.json'); +const reqOneOrg = require('./fixtures/reqOneOrg.json'); +const reqNoOrgs = require('./fixtures/reqNoOrgs.json'); + +const should = chai.should(); +chai.use(chaiHttp); +const requestutils = require('../rest/requestutils.js'); + +describe('requestutils().orgsArrayToString should return empty string', () => { + const emptyString = requestutils.orgsArrayToString(reqNoOrgs); + it('requestutils().orgsArrayToString should return empty string', () => { + assert.equal('', emptyString); + }); +}); + +describe('requestutils().orgsArrayToString should return single quotes value', () => { + const oneOrgString = requestutils.orgsArrayToString(reqOneOrg); + it('requestutils().orgsArrayToString should return single quotes value', () => { + assert.equal("'OrgOne'", oneOrgString); + }); +}); + +describe('requestutils().orgsArrayToString should return comma separated single quotes values ', () => { + const multiOrgsString = requestutils.orgsArrayToString(reqMultiOrgs); + const multiOrgs = "'OrdererMSP','Org2MSP'"; + it('requestutils().orgsArrayToString should return comma separated single quotes values ', () => { + assert.equal(multiOrgs, multiOrgsString); + }); +}); diff --git a/lgtm.yml b/lgtm.yml index 706954da9..e02577a2a 100644 --- a/lgtm.yml +++ b/lgtm.yml @@ -1,3 +1,4 @@ + # SPDX-License-Identifier: Apache-2.0 @@ -7,6 +8,7 @@ queries: - exclude: "js/stack-trace-exposure" - exclude: "js/useless-assignment-to-local" - exclude: "js/react/unused-or-undefined-state-property" + - exclude: "js/superfluous-trailing-arguments" extraction: javascript: index: diff --git a/package-lock.json b/package-lock.json index 8b28cefd8..17e44fd6c 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1122,8 +1122,7 @@ "decode-uri-component": { "version": "0.2.0", "resolved": "https://registry.npmjs.org/decode-uri-component/-/decode-uri-component-0.2.0.tgz", - "integrity": "sha1-6zkTMzRYd1y4TNGh+uBiEGu4dUU=", - "dev": true + "integrity": "sha1-6zkTMzRYd1y4TNGh+uBiEGu4dUU=" }, "dedent": { "version": "0.7.0", @@ -6251,6 +6250,16 @@ "resolved": "https://registry.npmjs.org/qs/-/qs-6.5.2.tgz", "integrity": "sha512-N5ZAX4/LxJmF+7wN74pUD6qAh9/wnvdQcjq9TZjevvXzSUo7bfmw91saqMjzGS2xq91/odN2dW/WOl7qQHNDGA==" }, + "query-string": { + "version": "6.9.0", + "resolved": "https://registry.npmjs.org/query-string/-/query-string-6.9.0.tgz", + "integrity": "sha512-KG4bhCFYapExLsUHrFt+kQVEegF2agm4cpF/VNc6pZVthIfCc/GK8t8VyNIE3nyXG9DK3Tf2EGkxjR6/uRdYsA==", + "requires": { + "decode-uri-component": "^0.2.0", + "split-on-first": "^1.0.0", + "strict-uri-encode": "^2.0.0" + } + }, "querystring": { "version": "0.2.0", "resolved": "https://registry.npmjs.org/querystring/-/querystring-0.2.0.tgz", @@ -6777,6 +6786,11 @@ "through": "2" } }, + "split-on-first": { + "version": "1.1.0", + "resolved": "https://registry.npmjs.org/split-on-first/-/split-on-first-1.1.0.tgz", + "integrity": "sha512-43ZssAJaMusuKWL8sKUBQXHWOpq8d6CfN/u1p4gUzfJkM05C8rxTmYrkIPTXapZpORA6LkkzcUulJ8FqA7Uudw==" + }, "split-string": { "version": "3.1.0", "resolved": "https://registry.npmjs.org/split-string/-/split-string-3.1.0.tgz", @@ -6904,6 +6918,11 @@ "resolved": "https://registry.npmjs.org/streamsearch/-/streamsearch-0.1.2.tgz", "integrity": "sha1-gIudDlb8Jz2Am6VzOOkpkZoanxo=" }, + "strict-uri-encode": { + "version": "2.0.0", + "resolved": "https://registry.npmjs.org/strict-uri-encode/-/strict-uri-encode-2.0.0.tgz", + "integrity": "sha1-ucczDHBChi9rFC3CdLvMWGbONUY=" + }, "string-argv": { "version": "0.0.2", "resolved": "https://registry.npmjs.org/string-argv/-/string-argv-0.0.2.tgz", diff --git a/package.json b/package.json index 1b2b496dd..d4c75b06f 100644 --- a/package.json +++ b/package.json @@ -45,6 +45,7 @@ "pg-pool": "^2.0.7", "prettyjson": "^1.2.1", "prop-types": "^15.6.2", + "query-string": "^6.9.0", "save": "^2.3.3", "stomp-broker-js": "^0.1.3", "stompjs": "^2.3.3",