Skip to content

Commit

Permalink
static analysis, this on local calls
Browse files Browse the repository at this point in the history
  • Loading branch information
Michael Fröwis committed Mar 21, 2017
1 parent 00e2a95 commit 3a720bf
Show file tree
Hide file tree
Showing 7 changed files with 193 additions and 3 deletions.
3 changes: 2 additions & 1 deletion src/app/staticanalysis/modules/list.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
module.exports = [
require('./txOrigin'),
require('./gasCosts')
require('./gasCosts'),
require('./thisLocal')
]
30 changes: 30 additions & 0 deletions src/app/staticanalysis/modules/thisLocal.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
var name = 'this on local'
var desc = 'Invocation of local functions via this'
var categories = require('./categories')
var common = require('./staticAnalysisCommon')

function thisLocal () {
this.warningNodes = []
}

thisLocal.prototype.visit = function (node) {
if (common.isThisLocalCall(node)) this.warningNodes.push(node)
}

thisLocal.prototype.report = function (compilationResults) {
this.warningNowNodes = []
return this.warningNodes.map(function (item, i) {
return {
warning: `use of "this" for local functions: never use this to call local functions, it only consumes more gas than normal local calls.`,
location: item.src,
more: 'http://solidity.readthedocs.io/en/develop/control-structures.html#external-function-calls'
}
})
}

module.exports = {
name: name,
description: desc,
category: categories.GAS,
Module: thisLocal
}
2 changes: 1 addition & 1 deletion src/app/staticanalysis/staticAnalysisView.js
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ staticAnalysisView.prototype.run = function () {
location = self.appAPI.offsetToLineColumn(location, file)
location = self.lastCompilationResult.sourceList[file] + ':' + (location.start.line + 1) + ':' + (location.start.column + 1) + ':'
}
self.appAPI.renderWarning(location + ' ' + item.warning, warningContainer, {type: 'warning', useSpan: true, isHTML: true})
self.appAPI.renderWarning(location + ' ' + item.warning + ((item.more) ? '<br><a href="' + item.more + '" target="blank">more</a>' : ''), warningContainer, {type: 'warning', useSpan: true, isHTML: true})
})
})
if (warningContainer.html() === '') {
Expand Down
12 changes: 11 additions & 1 deletion src/app/utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,17 @@ function groupBy (arr, key) {
}, {})
}

function concatWithSeperator (list, seperator) {
return list.reduce((sum, item) => sum + item + seperator, '').slice(0, -seperator.length)
}

function escapeRegExp (str) {
return str.replace(/[-[\]/{}()+?.\\^$|]/g, '\\$&')
}

module.exports = {
errortype: errortype,
groupBy: groupBy
groupBy: groupBy,
concatWithSeperator: concatWithSeperator,
escapeRegExp: escapeRegExp
}
1 change: 1 addition & 0 deletions test/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,3 +4,4 @@ require('./compiler-test')
require('./gist-handler-test')
require('./query-params-test')
require('./util-test')
require('./staticanalysis/staticAnalysisCommon-test')
132 changes: 132 additions & 0 deletions test/staticanalysis/staticAnalysisCommon-test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,132 @@
var test = require('tape')

var common = require('../../babelify-src/app/staticanalysis/modules/staticAnalysisCommon')
var utils = require('../../babelify-src/app/utils')

test('staticAnalysisCommon.helpers.buildFunctionSignature', function (t) {
t.plan(7)

t.equal(common.helpers.buildFunctionSignature([common.basicTypes.UINT, common.basicTypes.ADDRESS], [common.basicTypes.BOOL], false),
'function (uint256,address) returns (bool)',
'two params and return value without payable')

t.equal(common.helpers.buildFunctionSignature([common.basicTypes.UINT, common.basicTypes.BYTES32, common.basicTypes.BYTES32], [], true),
'function (uint256,bytes32,bytes32) payable',
'three params and no return with payable')

t.equal(common.helpers.buildFunctionSignature([common.basicTypes.BOOL], [common.basicTypes.BYTES32, common.basicTypes.ADDRESS], true),
'function (bool) payable returns (bytes32,address)',
'one param and two return values with payable')

t.equal(common.lowLevelCallTypes.CALL.type,
'function () payable returns (bool)',
'check fixed call type')

t.equal(common.lowLevelCallTypes.CALLCODE.type,
'function () payable returns (bool)',
'check fixed callcode type')

t.equal(common.lowLevelCallTypes.SEND.type,
'function (uint256) returns (bool)',
'check fixed send type')

t.equal(common.lowLevelCallTypes.DELEGATECALL.type,
'function () returns (bool)',
'check fixed call type')
})

test('staticAnalysisCommon.helpers.name', function (t) {
t.plan(9)
var node = { attributes: { value: 'now' } }
var node2 = { attributes: { member_name: 'call' } }

t.ok(common.helpers.name(node, 'now'), 'should work for values')
t.ok(common.helpers.name(node2, 'call'), 'should work for member_name')
t.ok(common.helpers.name(node2, '.all'), 'regex should work')

lowlevelAccessersCommon(t, common.helpers.name, node)
})

test('staticAnalysisCommon.helpers.nodeType', function (t) {
t.plan(9)
var node = { name: 'Identifier', attributes: { name: 'now' } }
var node2 = { name: 'FunctionCall', attributes: { member_name: 'call' } }

t.ok(common.helpers.nodeType(node, common.nodeTypes.IDENTIFIER), 'should work for ident')
t.ok(common.helpers.nodeType(node2, common.nodeTypes.FUNCTIONCALL), 'should work for funcall')
t.ok(common.helpers.nodeType(node2, '^F'), 'regex should work for funcall')

lowlevelAccessersCommon(t, common.helpers.nodeType, node)
})

test('staticAnalysisCommon.helpers.returnType', function (t) {
t.plan(9)
var node = { name: 'Identifier', attributes: { value: 'now', type: 'uint256' } }
var node2 = { name: 'FunctionCall', attributes: { member_name: 'call', type: 'function () payable returns (bool)' } }

t.ok(common.helpers.returnType(node, common.basicTypes.UINT), 'should work for ident')
t.ok(common.helpers.returnType(node2, utils.escapeRegExp(common.basicFunctionTypes.CALL)), 'should work for funcall')
t.ok(common.helpers.returnType(node2, '^function \\('), 'regex should work')

lowlevelAccessersCommon(t, common.helpers.returnType, node)
})

test('staticAnalysisCommon.helpers.nrOfChildren', function (t) {
t.plan(10)
var node = { name: 'Identifier', children: ['a', 'b'], attributes: { value: 'now', type: 'uint256' } }
var node2 = { name: 'FunctionCall', children: [], attributes: { member_name: 'call', type: 'function () payable returns (bool)' } }
var node3 = { name: 'FunctionCall', attributes: { member_name: 'call', type: 'function () payable returns (bool)' } }

t.ok(common.helpers.nrOfChildren(node, 2), 'should work for 2 children')
t.notOk(common.helpers.nrOfChildren(node, '1+2'), 'regex should not work')
t.ok(common.helpers.nrOfChildren(node2, 0), 'should work for 0 children')
t.notOk(common.helpers.nrOfChildren(node3, 0), 'should not work without children arr')

lowlevelAccessersCommon(t, common.helpers.nrOfChildren, node)
})

function lowlevelAccessersCommon (t, f, someNode) {
t.ok(f(someNode), 'always ok if type is undefinded')
t.ok(f(someNode, undefined), 'always ok if name is undefinded 2')
t.notOk(f(null, undefined), 'false on no node')
t.notOk(f(null, 'call'), 'false on no node')
t.notOk(f(undefined, null), 'false on no node')
t.notOk(f(), 'false on no params')
}

test('staticAnalysisCommon.helpers.isLowLevelCall', function (t) {
t.plan(4)
var sendAst = { name: 'MemberAccess', children: [{attributes: { value: 'd', type: 'address' }}], attributes: { value: 'send', type: 'function (uint256) returns (bool)' } }
var callAst = { name: 'MemberAccess', children: [{attributes: { value: 'f', type: 'address' }}], attributes: { member_name: 'call', type: 'function () payable returns (bool)' } }
var callcodeAst = { name: 'MemberAccess', children: [{attributes: { value: 'f', type: 'address' }}], attributes: { member_name: 'callcode', type: 'function () payable returns (bool)' } }
var delegatecallAst = { name: 'MemberAccess', children: [{attributes: { value: 'g', type: 'address' }}], attributes: { member_name: 'delegatecall', type: 'function () returns (bool)' } }

t.ok(common.isLowLevelSendInst(sendAst) && common.isLowLevelCall(sendAst), 'send is llc should work')
t.ok(common.isLowLevelCallInst(callAst) && common.isLowLevelCall(callAst), 'call is llc should work')
t.ok(common.isLowLevelCallcodeInst(callcodeAst) && common.isLowLevelCall(callcodeAst), 'callcode is llc should work')
t.ok(common.isLowLevelDelegatecallInst(delegatecallAst) && common.isLowLevelCall(delegatecallAst), 'delegatecall is llc should work')
})

test('staticAnalysisCommon.helpers.isThisLocalCall', function (t) {
t.plan(3)
var node = { name: 'MemberAccess', children: [{attributes: { value: 'this', type: 'contract test' }}], attributes: { value: 'b', type: 'function (bytes32,address) returns (bool)' } }
t.ok(common.isThisLocalCall(node), 'is this.local_method() used should work')
t.notOk(common.isBlockTimestampAccess(node), 'is block.timestamp used should not work')
t.notOk(common.isNowAccess(node), 'is now used should not work')
})

test('staticAnalysisCommon.helpers.isBlockTimestampAccess', function (t) {
t.plan(3)
var node = { name: 'MemberAccess', children: [{attributes: { value: 'block', type: 'block' }}], attributes: { value: 'timestamp', type: 'uint256' } }
t.notOk(common.isThisLocalCall(node), 'is this.local_method() used should not work')
t.ok(common.isBlockTimestampAccess(node), 'is block.timestamp used should work')
t.notOk(common.isNowAccess(node), 'is now used should not work')
})

test('staticAnalysisCommon.helpers.isNowAccess', function (t) {
t.plan(3)
var node = { name: 'Identifier', attributes: { value: 'now', type: 'uint256' } }
t.notOk(common.isThisLocalCall(node), 'is this.local_method() used should not work')
t.notOk(common.isBlockTimestampAccess(node), 'is block.timestamp used should not work')
t.ok(common.isNowAccess(node), 'is now used should work')
})
16 changes: 16 additions & 0 deletions test/util-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -24,3 +24,19 @@ test('util.groupBy on valid input', function (t) {

t.deepEqual(result, expectedResult)
})

test('util.concatWithSeperator valid output', function (t) {
t.plan(4)
t.notEqual(utils.concatWithSeperator(['a', 'b', 'c'], ','), 'a, b, c', 'Concat with comma should not produce spaces')
t.equal(utils.concatWithSeperator(['a', 'b', 'c'], ','), 'a,b,c', 'Concat with comma should not produce spaces')
t.equal(utils.concatWithSeperator(['a', 'b', 'c'], ', '), 'a, b, c', 'Concat with comma space should not produce trailing comma')
t.equal(utils.concatWithSeperator(['a', 'b', 'c'], '+'), 'a+b+c', 'Concat with plus')
})

test('util.escapeRegExp', function (t) {
t.plan(3)
var original = 'function (uint256) returns (bool)'
t.equal(utils.escapeRegExp('abcd'), 'abcd', 'String with no regex')
t.equal(utils.escapeRegExp(original), 'function \\(uint256\\) returns \\(bool\\)', 'function string with regex')
t.ok(new RegExp(utils.escapeRegExp(original)).test(original), 'should still test for original string')
})

0 comments on commit 3a720bf

Please sign in to comment.