Skip to content

Commit

Permalink
BE-717 Code quality fix (#66)
Browse files Browse the repository at this point in the history
* fix sql injection
* excluding warnings
* updated lgtm.yml syntax
* added code quality badge to README.md file

Signed-off-by: nfrunza <nfrunza@gmail.com>
  • Loading branch information
nfrunza authored and nekia committed Dec 8, 2019
1 parent 7cc3349 commit dac1dce
Show file tree
Hide file tree
Showing 6 changed files with 64 additions and 11 deletions.
1 change: 1 addition & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
[![Build Status](https://dev.azure.com/Hyperledger/blockchain-explorer/_apis/build/status/hyperledger.blockchain-explorer?branchName=master)](https://dev.azure.com/Hyperledger/blockchain-explorer/_build/latest?definitionId=41&branchName=master)
[![CII Best Practice](https://bestpractices.coreinfrastructure.org/projects/2710/badge)](https://bestpractices.coreinfrastructure.org/projects/2710)
[![Documentation Status](https://readthedocs.org/projects/blockchain-explorer/badge/?version=master)](https://blockchain-explorer.readthedocs.io/en/master/?badge=master)
[![Language grade: JavaScript](https://img.shields.io/lgtm/grade/javascript/g/hyperledger/blockchain-explorer.svg?logo=lgtm&logoWidth=18)](https://lgtm.com/projects/g/hyperledger/blockchain-explorer/context:javascript)

<!-- badges -->

Expand Down
15 changes: 15 additions & 0 deletions app/Explorer.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ const bodyParser = require('body-parser');
const swaggerUi = require('swagger-ui-express');
const compression = require('compression');
const passport = require('passport');
const RateLimit = require('express-rate-limit');
const PlatformBuilder = require('./platform/PlatformBuilder');
const explorerconfig = require('./explorerconfig.json');
const PersistenceFactory = require('./persistence/PersistenceFactory');
Expand Down Expand Up @@ -37,12 +38,26 @@ class Explorer {
*/
constructor() {
this.app = new Express();

// set up rate limiter: maximum of 1000 requests per minute

const limiter = new RateLimit({
windowMs: 1 * 60 * 1000, // 1 minute
max: 1000
});
// apply rate limiter to all requests
this.app.use(limiter);

this.app.use(bodyParser.json());
this.app.use(
bodyParser.urlencoded({
extended: true
})
);

// eslint-disable-next-line spellcheck/spell-checker
// handle rate limit, see https://lgtm.com/rules/1506065727959/

this.app.use(passport.initialize());
if (process.env.NODE_ENV !== 'production') {
this.app.use('/api-docs', swaggerUi.serve, swaggerUi.setup(swaggerDocument));
Expand Down
44 changes: 36 additions & 8 deletions app/persistence/fabric/CRUDService.js
Original file line number Diff line number Diff line change
Expand Up @@ -76,13 +76,25 @@ class CRUDService {
* @memberof CRUDService
*/
getTxList(channel_genesis_hash, blockNum, txid, from, to, orgs) {
let txListSql = '';
let byOrgs = false;
if (orgs && orgs !== '') {
txListSql = `and t.creator_msp_id in (${orgs})`;
byOrgs = true;
}

logger.debug('getTxList.byOrgs ', byOrgs);
console.debug('getTxList.byOrgs ', byOrgs);

const sqlTxListByOrgs = ` select t.creator_msp_id,t.txhash,t.type,t.chaincodename,t.createdt,channel.name as channelName from transactions as t
inner join channel on t.channel_genesis_hash=channel.channel_genesis_hash where t.blockid >= ${blockNum} and t.id >= ${txid} and t.creator_msp_id in (${orgs}) and
t.channel_genesis_hash = '${channel_genesis_hash}' and t.createdt between '${from}' and '${to}' order by t.id desc`;

const sqlTxList = ` select t.creator_msp_id,t.txhash,t.type,t.chaincodename,t.createdt,channel.name as channelName from transactions as t
inner join channel on t.channel_genesis_hash=channel.channel_genesis_hash where t.blockid >= ${blockNum} and t.id >= ${txid} ${txListSql} and
t.channel_genesis_hash = '${channel_genesis_hash}' and t.createdt between '${from}' and '${to}' order by t.id desc`;
inner join channel on t.channel_genesis_hash=channel.channel_genesis_hash where t.blockid >= ${blockNum} and t.id >= ${txid} and
t.channel_genesis_hash = '${channel_genesis_hash}' and t.createdt between '${from}' and '${to}' order by t.id desc`;

if (byOrgs) {
return this.sql.getRowsBySQlQuery(sqlTxListByOrgs);
}
return this.sql.getRowsBySQlQuery(sqlTxList);
}

Expand All @@ -99,17 +111,33 @@ class CRUDService {
* @memberof CRUDService
*/
getBlockAndTxList(channel_genesis_hash, blockNum, from, to, orgs) {
let blockTxListSql = '';
let byOrgs = false;
// workaround for SQL injection
if (orgs && orgs !== '') {
blockTxListSql = `and creator_msp_id in (${orgs})`;
byOrgs = true;
}

logger.debug('getBlockAndTxList.byOrgs ', byOrgs);
console.debug('getBlockAndTxList.byOrgs ', byOrgs);

const sqlBlockTxList = `select a.* from (
select (select c.name from channel c where c.channel_genesis_hash =
'${channel_genesis_hash}' ) as channelname, blocks.blocknum,blocks.txcount ,blocks.datahash ,blocks.blockhash ,blocks.prehash,blocks.createdt, blocks.blksize, (
SELECT array_agg(txhash) as txhash FROM transactions where blockid = blocks.blocknum ${blockTxListSql} and
SELECT array_agg(txhash) as txhash FROM transactions where blockid = blocks.blocknum and
channel_genesis_hash = '${channel_genesis_hash}' and createdt between '${from}' and '${to}') from blocks where
blocks.channel_genesis_hash ='${channel_genesis_hash}' and blocknum >= 0 and blocks.createdt between '${from}' and '${to}'
order by blocks.blocknum desc) a where a.txhash IS NOT NULL`;
order by blocks.blocknum desc) a where a.txhash IS NOT NULL`;

const sqlBlockTxListByOrgs = `select a.* from (
select (select c.name from channel c where c.channel_genesis_hash =
'${channel_genesis_hash}' ) as channelname, blocks.blocknum,blocks.txcount ,blocks.datahash ,blocks.blockhash ,blocks.prehash,blocks.createdt, blocks.blksize, (
SELECT array_agg(txhash) as txhash FROM transactions where blockid = blocks.blocknum and creator_msp_id in (${orgs}) and
channel_genesis_hash = '${channel_genesis_hash}' and createdt between '${from}' and '${to}') from blocks where
blocks.channel_genesis_hash ='${channel_genesis_hash}' and blocknum >= 0 and blocks.createdt between '${from}' and '${to}'
order by blocks.blocknum desc) a where a.txhash IS NOT NULL`;
if (byOrgs) {
return this.sql.getRowsBySQlQuery(sqlBlockTxListByOrgs);
}
return this.sql.getRowsBySQlQuery(sqlBlockTxList);
}

Expand Down
5 changes: 4 additions & 1 deletion lgtm.yml
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,10 @@

path_classifiers:
queries:
exclude: "js/unused-local-variable"
- exclude: "js/unused-local-variable"
- exclude: "js/stack-trace-exposure"
- exclude: "js/useless-assignment-to-local"
- exclude: "js/react/unused-or-undefined-state-property"
extraction:
javascript:
index:
Expand Down
7 changes: 6 additions & 1 deletion package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 2 additions & 1 deletion package.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"name": "hyperledger-explorer",
"version": "0.3.9",
"version": "1.0.0-rc1",
"description": "hyperledger-explorer",
"private": true,
"main": "main.js",
Expand All @@ -25,6 +25,7 @@
"ejs": "^2.5.6",
"enum": "^2.5.0",
"express": "^4.15.3",
"express-rate-limit": "^5.0.0",
"fabric-ca-client": "^1.4.4",
"fabric-client": "^1.4.4",
"fabric-network": "^1.4.4",
Expand Down

0 comments on commit dac1dce

Please sign in to comment.