BE-719 Fix parameter tampering (#68)

* BE-719 Fix parameter tampering Signed-off-by: nfrunza <[email protected]> * BE-719 Exclude warning * added js/superfluous-trailing-arguments to lgtm.yml config Signed-off-by: nfrunza <[email protected]> * BE-719 Fix parameter tampering * added tests, removed console.debug statements Signed-off-by: nfrunza <[email protected]> * BE-719 Fix parameter tampering * removed console.debug staement Signed-off-by: nfrunza <[email protected]>
hyperledger-labs · Dec 10, 2019 · 96134fa · 96134fa
1 parent 50d196d
commit 96134fa
Show file tree

Hide file tree

Showing 9 changed files with 100 additions and 13 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
diff --git 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",