From eb84b083b173b792bc27d571c0bb6a11350a3345 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michael=20Fr=C3=B6wis?= Date: Tue, 4 Apr 2017 12:11:27 +0200 Subject: [PATCH] Static Analysis: fix blockBlockhash, warn on modifiers --- .../modules/checksEffectsInteraction.js | 12 +-- .../modules/constantFunctions.js | 12 +-- .../modules/staticAnalysisCommon.js | 19 ++++- .../staticAnalysisCommon-test.js | 85 ++++++++++++++++--- .../staticAnalysisIntegration-test.js | 2 +- 5 files changed, 104 insertions(+), 26 deletions(-) diff --git a/src/app/staticanalysis/modules/checksEffectsInteraction.js b/src/app/staticanalysis/modules/checksEffectsInteraction.js index 7145e48d17..07ed9661a5 100644 --- a/src/app/staticanalysis/modules/checksEffectsInteraction.js +++ b/src/app/staticanalysis/modules/checksEffectsInteraction.js @@ -7,19 +7,19 @@ var AbstractAst = require('./abstractAstView') function checksEffectsInteraction () { this.contracts = [] -} -checksEffectsInteraction.prototype.visit = new AbstractAst().builder( - (node) => common.isInteraction(node) || common.isEffect(node) || common.isLocalCall(node), - this.contracts -) + checksEffectsInteraction.prototype.visit = new AbstractAst().builder( + (node) => common.isInteraction(node) || common.isEffect(node) || (common.isLocalCall(node) && !common.isBuiltinFunctionCall(node)), + this.contracts + ) +} checksEffectsInteraction.prototype.report = function (compilationResults) { var warnings = [] var cg = callGraph.buildGlobalFuncCallGraph(this.contracts) - if (this.contracts.some((item) => item.modifiers.length > 0)) this.warning.push({ warning: `Modifiers are currently not supported by this analysis.` }) + if (this.contracts.some((item) => item.modifiers.length > 0)) warnings.push({ warning: `Modifiers are currently not supported by the '${name}' static analysis.` }) this.contracts.forEach((contract) => { contract.functions.forEach((func) => { diff --git a/src/app/staticanalysis/modules/constantFunctions.js b/src/app/staticanalysis/modules/constantFunctions.js index 8f69e204c8..2075e47107 100644 --- a/src/app/staticanalysis/modules/constantFunctions.js +++ b/src/app/staticanalysis/modules/constantFunctions.js @@ -7,19 +7,19 @@ var AbstractAst = require('./abstractAstView') function constantFunctions () { this.contracts = [] -} -constantFunctions.prototype.visit = new AbstractAst().builder( - (node) => common.isLowLevelCall(node) || common.isExternalDirectCall(node) || common.isEffect(node) || common.isLocalCall(node) || common.isInlineAssembly(node), - this.contracts -) + constantFunctions.prototype.visit = new AbstractAst().builder( + (node) => common.isLowLevelCall(node) || common.isExternalDirectCall(node) || common.isEffect(node) || (common.isLocalCall(node) && !common.isBuiltinFunctionCall(node)) || common.isInlineAssembly(node), + this.contracts + ) +} constantFunctions.prototype.report = function (compilationResults) { var warnings = [] var cg = callGraph.buildGlobalFuncCallGraph(this.contracts) - if (this.contracts.some((item) => item.modifiers.length > 0)) this.warning.push({ warning: `Modifiers are currently not supported by this analysis.` }) + if (this.contracts.some((item) => item.modifiers.length > 0)) warnings.push({ warning: `Modifiers are currently not supported by the '${name}' static analysis.` }) this.contracts.forEach((contract) => { if (!common.isFullyImplementedContract(contract.node)) return diff --git a/src/app/staticanalysis/modules/staticAnalysisCommon.js b/src/app/staticanalysis/modules/staticAnalysisCommon.js index 8d01e8719e..db5df7bae5 100644 --- a/src/app/staticanalysis/modules/staticAnalysisCommon.js +++ b/src/app/staticanalysis/modules/staticAnalysisCommon.js @@ -41,6 +41,17 @@ var basicFunctionTypes = { DELEGATECALL: buildFunctionSignature([], [basicTypes.BOOL], false) } +var builtinFunctions = { + 'keccak256()': true, + 'sha3()': true, + 'sha256()': true, + 'ripemd160()': true, + 'ecrecover(bytes32,uint8,bytes32,bytes32)': true, + 'addmod(uint256,uint256,uint256)': true, + 'mulmod(uint256,uint256,uint256)': true, + 'selfdestruct(address)': true +} + var lowLevelCallTypes = { CALL: { ident: 'call', type: basicFunctionTypes.CALL }, CALLCODE: { ident: 'callcode', type: basicFunctionTypes.CALL }, @@ -182,6 +193,10 @@ function isInlineAssembly (node) { // #################### Complex Node Identification +function isBuiltinFunctionCall (node) { + return isLocalCall(node) && builtinFunctions[getLocalCallName(node) + '(' + getFunctionCallTypeParameterType(node) + ')'] === true +} + function isStorageVariableDeclaration (node) { return isVariableDeclaration(node) && expressionType(node, basicRegex.REFTYPE) } @@ -297,7 +312,7 @@ function isMemberAccess (node, retType, accessor, accessorType, memberName) { } function isSpecialVariableAccess (node, varType) { - return isMemberAccess(node, varType.type, varType.obj, varType.obj, varType.member) + return isMemberAccess(node, exactMatch(utils.escapeRegExp(varType.type)), varType.obj, varType.obj, varType.member) } // #################### Node Identification Primitives @@ -340,7 +355,6 @@ function exactMatch (regexStr) { * @param {Array} returnTypes * list of return type names * @return {Boolean} isPayable - * CAUTION: only needed in low level call signature or message-calls (other contracts, this.) */ function buildFunctionSignature (paramTypes, returnTypes, isPayable) { return 'function (' + utils.concatWithSeperator(paramTypes, ',') + ')' + ((isPayable) ? ' payable' : '') + ((returnTypes.length) ? ' returns (' + utils.concatWithSeperator(returnTypes, ',') + ')' : '') @@ -400,6 +414,7 @@ module.exports = { isCallToNonConstLocalFunction: isCallToNonConstLocalFunction, isPlusPlusUnaryOperation: isPlusPlusUnaryOperation, isMinusMinusUnaryOperation: isMinusMinusUnaryOperation, + isBuiltinFunctionCall: isBuiltinFunctionCall, // #################### Trivial Node Identification isFunctionDefinition: isFunctionDefinition, diff --git a/test/staticanalysis/staticAnalysisCommon-test.js b/test/staticanalysis/staticAnalysisCommon-test.js index cb2396f7be..966fc0e25b 100644 --- a/test/staticanalysis/staticAnalysisCommon-test.js +++ b/test/staticanalysis/staticAnalysisCommon-test.js @@ -1066,6 +1066,59 @@ test('staticAnalysisCommon.isInlineAssembly', function (t) { // #################### Complex Node Identification +test('staticAnalysisCommon.isBuiltinFunctionCall', function (t) { + t.plan(2) + var selfdestruct = { + 'attributes': { + 'type': 'tuple()', + 'type_conversion': false + }, + 'children': [ + { + 'attributes': { + 'type': 'function (address)', + 'value': 'selfdestruct' + }, + 'name': 'Identifier' + }, + { + 'attributes': { + 'type': 'address', + 'value': 'a' + }, + 'name': 'Identifier' + } + ], + 'name': 'FunctionCall' + } + var localCall = { + 'attributes': { + 'type': 'tuple()', + 'type_conversion': false + }, + 'children': [ + { + 'attributes': { + 'type': 'function (struct Ballot.Voter storage pointer)', + 'value': 'bli' + }, + 'name': 'Identifier' + }, + { + 'attributes': { + 'type': 'struct Ballot.Voter storage pointer', + 'value': 'x' + }, + 'name': 'Identifier' + } + ], + 'name': 'FunctionCall' + } + + t.ok(common.isBuiltinFunctionCall(selfdestruct), 'selfdestruct is builtin') + t.notOk(common.isBuiltinFunctionCall(localCall), 'local call is not builtin') +}) + test('staticAnalysisCommon.isStorageVariableDeclaration', function (t) { t.plan(3) var node1 = { @@ -1522,23 +1575,17 @@ test('staticAnalysisCommon.isCallToNonConstLocalFunction', function (t) { 'type': 'function (struct Ballot.Voter storage pointer)', 'value': 'bli' }, - 'id': 37, - 'name': 'Identifier', - 'src': '540:3:0' + 'name': 'Identifier' }, { 'attributes': { 'type': 'struct Ballot.Voter storage pointer', 'value': 'x' }, - 'id': 38, - 'name': 'Identifier', - 'src': '544:1:0' + 'name': 'Identifier' } ], - 'id': 39, - 'name': 'FunctionCall', - 'src': '540:6:0' + 'name': 'FunctionCall' } t.ok(common.isCallToNonConstLocalFunction(node1), 'should be call to non const Local func') @@ -1595,10 +1642,26 @@ test('staticAnalysisCommon.isBlockTimestampAccess', function (t) { test('staticAnalysisCommon.isBlockBlockhashAccess', function (t) { t.plan(4) - var node = { name: 'MemberAccess', children: [{attributes: { value: 'block', type: 'block' }}], attributes: { value: 'blockhash', type: 'bytes32' } } + var node = { + 'attributes': { + 'member_name': 'blockhash', + 'type': 'function (uint256) returns (bytes32)' + }, + 'children': [ + { + 'attributes': { + 'type': 'block', + 'value': 'block' + }, + 'name': 'Identifier' + } + ], + 'name': 'MemberAccess' + } + 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.isBlockBlockHashAccess(node), 'blockhash should work') + t.ok(common.isBlockBlockHashAccess(node), 'blockhash should work') // todo: t.notOk(common.isNowAccess(node), 'is now used should not work') }) diff --git a/test/staticanalysis/staticAnalysisIntegration-test.js b/test/staticanalysis/staticAnalysisIntegration-test.js index 036d212f6d..da0adba732 100644 --- a/test/staticanalysis/staticAnalysisIntegration-test.js +++ b/test/staticanalysis/staticAnalysisIntegration-test.js @@ -1,6 +1,6 @@ var test = require('tape') -var StatRunner = require('../../babelify-src/app/staticanalysis/staticAnalysisRunner') +var StatRunner = require('../../src/app/staticanalysis/staticAnalysisRunner') // const util = require('util') var solc = require('solc')