From 11917ffcfb5dc3b0cfba190a8849d4d22b6e659e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michael=20Fr=C3=B6wis?= Date: Thu, 30 Mar 2017 11:23:52 +0200 Subject: [PATCH 01/18] Static Analysis: ChecksEffectsInteraction, Constant Function, Inline Assembly. Without integration tests and modifier support --- .../staticanalysis/modules/abstractAstView.js | 93 ++++++ src/app/staticanalysis/modules/categories.js | 3 +- .../modules/checksEffectsInteraction.js | 81 +++++ .../modules/constantFunctions.js | 86 ++++++ .../modules/functionCallGraph.js | 84 +++++ .../staticanalysis/modules/inlineAssembly.js | 29 ++ src/app/staticanalysis/modules/list.js | 5 +- .../modules/staticAnalysisCommon.js | 290 +++++++++++++++++- src/app/staticanalysis/modules/thisLocal.js | 1 - .../staticanalysis/staticAnalysisRunner.js | 4 + test/index.js | 1 + .../staticAnalysisCommon-test.js | 45 ++- .../staticAnalysisIntegration-test.js | 47 +++ .../test-contracts/KingOfTheEtherThrone.sol | 23 ++ .../test-contracts/assembly.sol | 26 ++ test/staticanalysis/test-contracts/ballot.sol | 145 +++++++++ .../test-contracts/ballot_reentrant.sol | 101 ++++++ .../test-contracts/ballot_withoutWarnings.sol | 31 ++ .../test-contracts/cross_contract.sol | 19 ++ .../test-contracts/inheritance.sol | 40 +++ .../test-contracts/modifier1.sol | 17 + .../test-contracts/modifier2.sol | 28 ++ .../test-contracts/notReentrant.sol | 13 + .../test-contracts/reentrant.sol | 49 +++ .../test-contracts/structReentrant.sol | 36 +++ .../test-contracts/thisLocal.sol | 16 + 26 files changed, 1288 insertions(+), 25 deletions(-) create mode 100644 src/app/staticanalysis/modules/abstractAstView.js create mode 100644 src/app/staticanalysis/modules/checksEffectsInteraction.js create mode 100644 src/app/staticanalysis/modules/constantFunctions.js create mode 100644 src/app/staticanalysis/modules/functionCallGraph.js create mode 100644 src/app/staticanalysis/modules/inlineAssembly.js create mode 100644 test/staticanalysis/staticAnalysisIntegration-test.js create mode 100644 test/staticanalysis/test-contracts/KingOfTheEtherThrone.sol create mode 100644 test/staticanalysis/test-contracts/assembly.sol create mode 100644 test/staticanalysis/test-contracts/ballot.sol create mode 100644 test/staticanalysis/test-contracts/ballot_reentrant.sol create mode 100644 test/staticanalysis/test-contracts/ballot_withoutWarnings.sol create mode 100644 test/staticanalysis/test-contracts/cross_contract.sol create mode 100644 test/staticanalysis/test-contracts/inheritance.sol create mode 100644 test/staticanalysis/test-contracts/modifier1.sol create mode 100644 test/staticanalysis/test-contracts/modifier2.sol create mode 100644 test/staticanalysis/test-contracts/notReentrant.sol create mode 100644 test/staticanalysis/test-contracts/reentrant.sol create mode 100644 test/staticanalysis/test-contracts/structReentrant.sol create mode 100644 test/staticanalysis/test-contracts/thisLocal.sol diff --git a/src/app/staticanalysis/modules/abstractAstView.js b/src/app/staticanalysis/modules/abstractAstView.js new file mode 100644 index 0000000000..7f2a610a45 --- /dev/null +++ b/src/app/staticanalysis/modules/abstractAstView.js @@ -0,0 +1,93 @@ +var common = require('./staticAnalysisCommon') +var AstWalker = require('ethereum-remix').util.AstWalker + +function abstractAstView () { + this.contracts = null + this.currentContractIndex = null + this.currentFunctionIndex = null + this.currentModifierIndex = null + this.isFunctionNotModifier = false +} + +abstractAstView.prototype.builder = function (relevantNodeFilter, contractsOut) { + this.contracts = contractsOut + return function (node) { + if (common.isContractDefinition(node)) { + setCurrentContract(this, { + node: node, + functions: [], + modifiers: [], + inheritsFrom: [], + stateVariables: common.getStateVariableDeclarationsFormContractNode(node) + }) + } else if (common.isInheritanceSpecifier(node)) { + var currentContract = getCurrentContract(this) + var inheritsFromName = common.getInheritsFromName(node) + currentContract.inheritsFrom.push(inheritsFromName) + // add variables from inherited contracts + var inheritsFrom = this.contracts.find((contract) => common.getContractName(contract.node) === inheritsFromName) + currentContract.stateVariables = currentContract.stateVariables.concat(inheritsFrom.stateVariables) + } else if (common.isFunctionDefinition(node)) { + setCurrentFunction(this, { + node: node, + relevantNodes: [], + modifierInvocations: [], + localVariables: getLocalVariables(node), + parameters: getLocalParameters(node) + }) + } else if (common.isModifierDefinition(node)) { + setCurrentModifier(this, { + node: node, + relevantNodes: [], + localVariables: getLocalVariables(node), + parameters: getLocalParameters(node) + }) + } else if (common.isModifierInvocation(node)) { + if (!this.isFunctionNotModifier) throw new Error('abstractAstView.js: Found modifier invocation outside of function scope.') + getCurrentFunction(this).modifierInvocations.push(node) + } else if (relevantNodeFilter(node)) { + ((this.isFunctionNotModifier) ? getCurrentFunction(this) : getCurrentModifier(this)).relevantNodes.push(node) + } + } +} + +function setCurrentContract (that, contract) { + that.currentContractIndex = (that.contracts.push(contract) - 1) +} + +function setCurrentFunction (that, func) { + that.isFunctionNotModifier = true + that.currentFunctionIndex = (getCurrentContract(that).functions.push(func) - 1) +} + +function setCurrentModifier (that, modi) { + that.isFunctionNotModifier = false + that.currentModifierIndex = (getCurrentContract(that).modifiers.push(modi) - 1) +} + +function getCurrentContract (that) { + return that.contracts[that.currentContractIndex] +} + +function getCurrentFunction (that) { + return getCurrentContract(that).functions[that.currentFunctionIndex] +} + +function getCurrentModifier (that) { + return getCurrentContract(that).modifiers[that.currentModifierIndex] +} + +function getLocalParameters (funcNode) { + return getLocalVariables(common.getFunctionOrModifierDefinitionParameterPart(funcNode)).map(common.getType) +} + +function getLocalVariables (funcNode) { + var locals = [] + new AstWalker().walk(funcNode, {'*': function (node) { + if (common.isVariableDeclaration(node)) locals.push(node) + return true + }}) + return locals +} + +module.exports = abstractAstView diff --git a/src/app/staticanalysis/modules/categories.js b/src/app/staticanalysis/modules/categories.js index 380f54b534..2b0e8d3414 100644 --- a/src/app/staticanalysis/modules/categories.js +++ b/src/app/staticanalysis/modules/categories.js @@ -1,4 +1,5 @@ module.exports = { SECURITY: {displayName: 'Security', id: 'SEC'}, - GAS: {displayName: 'Gas & Economy', id: 'GAS'} + GAS: {displayName: 'Gas & Economy', id: 'GAS'}, + MISC: {displayName: 'Miscellaneous', id: 'MISC'} } diff --git a/src/app/staticanalysis/modules/checksEffectsInteraction.js b/src/app/staticanalysis/modules/checksEffectsInteraction.js new file mode 100644 index 0000000000..86f60a2545 --- /dev/null +++ b/src/app/staticanalysis/modules/checksEffectsInteraction.js @@ -0,0 +1,81 @@ +var name = 'Checks-Effects-Interaction pattern' +var desc = 'Avoid potential reentrancy bugs' +var categories = require('./categories') +var common = require('./staticAnalysisCommon') +var callGraph = require('./functionCallGraph') +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.report = function (compilationResults) { + var warnings = [] + + var cg = callGraph.buildGlobalFuncCallGraph(this.contracts) + + this.contracts.forEach((contract) => { + contract.functions.forEach((func) => { + func.changesState = checkIfChangesState(common.getFullQuallyfiedFuncDefinitionIdent(contract.node, func.node, func.parameters), + getContext(cg, contract, func)) + }) + + contract.functions.forEach((func) => { + if (isPotentialVulnerableFunction(func, getContext(cg, contract, func))) { + var funcName = common.getFullQuallyfiedFuncDefinitionIdent(contract.node, func.node, func.parameters) + warnings.push({ + warning: `Potential Violation of Checks-Effects-Interaction pattern in ${funcName}: Could potentially lead to re-entrancy vulnerability.`, + location: func.src, + more: 'http://solidity.readthedocs.io/en/develop/security-considerations.html#re-entrancy' + }) + } + }) + }) + + return warnings +} + +function getContext (cg, currentContract, func) { + return { cg: cg, currentContract: currentContract, stateVariables: getStateVariables(currentContract, func) } +} + +function getStateVariables (contract, func) { + return contract.stateVariables.concat(func.localVariables.filter(common.isStorageVariableDeclaration)) +} + +function isPotentialVulnerableFunction (func, context) { + var isPotentialVulnerable = false + var interaction = false + func.relevantNodes.forEach((node) => { + if (common.isInteraction(node)) { + interaction = true + } else if (interaction && (common.isWriteOnStateVariable(node, context.stateVariables) || isLocalCallWithStateChange(node, context))) { + isPotentialVulnerable = true + } + }) + return isPotentialVulnerable +} + +function isLocalCallWithStateChange (node, context) { + if (common.isLocalCall(node)) { + var func = callGraph.resolveCallGraphSymbol(context.cg, common.getFullQualifiedFunctionCallIdent(context.currentContract.node, node)) + return !func || (func && func.node.changesState) + } + return false +} + +function checkIfChangesState (startFuncName, context) { + return callGraph.analyseCallGraph(context.cg, startFuncName, context, (node, context) => common.isWriteOnStateVariable(node, context.stateVariables)) +} + +module.exports = { + name: name, + description: desc, + category: categories.SECURITY, + Module: checksEffectsInteraction +} diff --git a/src/app/staticanalysis/modules/constantFunctions.js b/src/app/staticanalysis/modules/constantFunctions.js new file mode 100644 index 0000000000..c1c95a9399 --- /dev/null +++ b/src/app/staticanalysis/modules/constantFunctions.js @@ -0,0 +1,86 @@ +var name = 'Constant functions' +var desc = 'Check for potentially constant functions' +var categories = require('./categories') +var common = require('./staticAnalysisCommon') +var callGraph = require('./functionCallGraph') +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.report = function (compilationResults) { + var warnings = [] + + var cg = callGraph.buildGlobalFuncCallGraph(this.contracts) + + this.contracts.forEach((contract) => { + if (!common.isFullyImplementedContract(contract.node)) return + + contract.functions.forEach((func) => { + func.potentiallyshouldBeConst = checkIfShouldBeConstant(common.getFullQuallyfiedFuncDefinitionIdent(contract.node, func.node, func.parameters), + getContext(cg, contract, func)) + }) + + contract.functions.forEach((func) => { + if (common.isConstantFunction(func.node) !== func.potentiallyshouldBeConst) { + var funcName = common.getFullQuallyfiedFuncDefinitionIdent(contract.node, func.node, func.parameters) + if (func.potentiallyshouldBeConst) { + warnings.push({ + warning: `${funcName}: Potentially should be constant but is not.`, + location: func.src, + more: 'http://solidity.readthedocs.io/en/develop/contracts.html#constant-functions' + }) + } else { + warnings.push({ + warning: `${funcName}: Is constant but potentially should not be.`, + location: func.src, + more: 'http://solidity.readthedocs.io/en/develop/contracts.html#constant-functions' + }) + } + } + }) + }) + + return warnings +} + +function getContext (cg, currentContract, func) { + return { cg: cg, currentContract: currentContract, stateVariables: getStateVariables(currentContract, func) } +} + +function getStateVariables (contract, func) { + return contract.stateVariables.concat(func.localVariables.filter(common.isStorageVariableDeclaration)) +} + +function checkIfShouldBeConstant (startFuncName, context) { + return !callGraph.analyseCallGraph(context.cg, startFuncName, context, isConstBreaker) +} + +function isConstBreaker (node, context) { + return common.isWriteOnStateVariable(node, context.stateVariables) || + common.isLowLevelCall(node) || + isCallOnNonConstExternalInterfaceFunction(node, context) || + common.isCallToNonConstLocalFunction(node) || + common.isInlineAssembly(node) +} + +function isCallOnNonConstExternalInterfaceFunction (node, context) { + if (common.isExternalDirectCall(node)) { + var func = callGraph.resolveCallGraphSymbol(context.cg, common.getFullQualifiedFunctionCallIdent(context.currentContract, node)) + return !func || (func && !common.isConstantFunction(func.node.node)) + } + return false +} + +module.exports = { + name: name, + description: desc, + category: categories.MISC, + Module: constantFunctions +} diff --git a/src/app/staticanalysis/modules/functionCallGraph.js b/src/app/staticanalysis/modules/functionCallGraph.js new file mode 100644 index 0000000000..1e52da7593 --- /dev/null +++ b/src/app/staticanalysis/modules/functionCallGraph.js @@ -0,0 +1,84 @@ +'use strict' + +var common = require('./staticAnalysisCommon') + +function buildLocalFuncCallGraphInternal (functions, nodeFilter, extractNodeIdent, extractFuncDefIdent) { + var callGraph = {} + functions.forEach((func) => { + var calls = func.relevantNodes + .filter(nodeFilter) + .map(extractNodeIdent) + .filter((name) => name !== extractFuncDefIdent(func)) // filter self recursive call + + callGraph[extractFuncDefIdent(func)] = { node: func, calls: calls } + }) + + return callGraph +} + +function buildGlobalFuncCallGraph (contracts) { + var callGraph = {} + contracts.forEach((contract) => { + var filterNodes = (node) => { return common.isLocalCall(node) || common.isThisLocalCall(node) || common.isExternalDirectCall(node) } + var getNodeIdent = (node) => { return common.getFullQualifiedFunctionCallIdent(contract.node, node) } + var getFunDefIdent = (funcDef) => { return common.getFullQuallyfiedFuncDefinitionIdent(contract.node, funcDef.node, funcDef.parameters) } + + callGraph[common.getContractName(contract.node)] = { contract: contract, functions: buildLocalFuncCallGraphInternal(contract.functions, filterNodes, getNodeIdent, getFunDefIdent) } + }) + + return callGraph +} + +function analyseCallGraph (cg, funcName, context, nodeCheck) { + return analyseCallGraphInternal(cg, funcName, context, (a, b) => a || b, nodeCheck, {}) +} + +function analyseCallGraphInternal (cg, funcName, context, combinator, nodeCheck, visited) { + var current = resolveCallGraphSymbol(cg, funcName) + + if (!current || visited[funcName]) return true + visited[funcName] = true + + return combinator(current.node.relevantNodes.reduce((acc, val) => combinator(acc, nodeCheck(val, context)), false), + current.calls.reduce((acc, val) => combinator(acc, analyseCallGraphInternal(cg, val, context, combinator, nodeCheck, visited)), false)) +} + +function resolveCallGraphSymbol (cg, funcName) { + return resolveCallGraphSymbolInternal(cg, funcName, false) +} + +function resolveCallGraphSymbolInternal (cg, funcName, silent) { + var current = null + if (funcName.includes('.')) { + var parts = funcName.split('.') + var contractPart = parts[0] + var functionPart = parts[1] + var currentContract = cg[contractPart] + if (currentContract) { + current = currentContract.functions[funcName] + // resolve inheritance hierarchy + if (!current) { + // resolve inheritance lookup in linearized fashion + var inheritsFromNames = currentContract.contract.inheritsFrom.reverse() + for (var i = 0; i < inheritsFromNames.length; i++) { + var res = resolveCallGraphSymbolInternal(cg, inheritsFromNames[i] + '.' + functionPart, true) + if (res) return res + } + } + } + } else { + throw new Error('functionCallGraph.js: function does not have full qualified name.') + } + if (!current) { + if (!silent) console.log(`static analysis functionCallGraph.js: ${funcName} not found in function call graph.`) + return null + } else { + return current + } +} + +module.exports = { + analyseCallGraph: analyseCallGraph, + buildGlobalFuncCallGraph: buildGlobalFuncCallGraph, + resolveCallGraphSymbol: resolveCallGraphSymbol +} diff --git a/src/app/staticanalysis/modules/inlineAssembly.js b/src/app/staticanalysis/modules/inlineAssembly.js new file mode 100644 index 0000000000..858ace52f4 --- /dev/null +++ b/src/app/staticanalysis/modules/inlineAssembly.js @@ -0,0 +1,29 @@ +var name = 'Inline Assembly' +var desc = 'Use of Inline Assembly' +var categories = require('./categories') +var common = require('./staticAnalysisCommon') + +function inlineAssembly () { + this.inlineAssNodes = [] +} + +inlineAssembly.prototype.visit = function (node) { + if (common.isInlineAssembly(node)) this.inlineAssNodes.push(node) +} + +inlineAssembly.prototype.report = function (compilationResults) { + return this.inlineAssNodes.map((node) => { + return { + warning: `CAUTION: The Contract uses inline assembly, this is only advised in rare cases. Additionally static analysis modules do not parse inline Assembly, this can lead to wrong analysis results.`, + location: node.src, + more: 'http://solidity.readthedocs.io/en/develop/assembly.html#solidity-assembly' + } + }) +} + +module.exports = { + name: name, + description: desc, + category: categories.SECURITY, + Module: inlineAssembly +} diff --git a/src/app/staticanalysis/modules/list.js b/src/app/staticanalysis/modules/list.js index 9442d596cf..57ed694688 100644 --- a/src/app/staticanalysis/modules/list.js +++ b/src/app/staticanalysis/modules/list.js @@ -1,5 +1,8 @@ module.exports = [ require('./txOrigin'), require('./gasCosts'), - require('./thisLocal') + require('./thisLocal'), + require('./checksEffectsInteraction'), + require('./constantFunctions'), + require('./inlineAssembly') ] diff --git a/src/app/staticanalysis/modules/staticAnalysisCommon.js b/src/app/staticanalysis/modules/staticAnalysisCommon.js index d27a5bd081..c0f94960e5 100644 --- a/src/app/staticanalysis/modules/staticAnalysisCommon.js +++ b/src/app/staticanalysis/modules/staticAnalysisCommon.js @@ -5,7 +5,18 @@ var utils = require('../../utils') var nodeTypes = { IDENTIFIER: 'Identifier', MEMBERACCESS: 'MemberAccess', - FUNCTIONCALL: 'FunctionCall' + FUNCTIONCALL: 'FunctionCall', + FUNCTIONDEFINITION: 'FunctionDefinition', + VARIABLEDECLARATION: 'VariableDeclaration', + ASSIGNMENT: 'Assignment', + CONTRACTDEFINITION: 'ContractDefinition', + UNARYOPERATION: 'UnaryOperation', + EXPRESSIONSTATEMENT: 'ExpressionStatement', + MODIFIERDEFINITION: 'ModifierDefinition', + MODIFIERINVOCATION: 'ModifierInvocation', + INHERITANCESPECIFIER: 'InheritanceSpecifier', + USERDEFINEDTYPENAME: 'UserDefinedTypeName', + INLINEASSEMBLY: 'InlineAssembly' } var basicTypes = { @@ -17,7 +28,11 @@ var basicTypes = { var basicRegex = { CONTRACTTYPE: '^contract ', - FUNCTIONTYPE: '^function \\(' + FUNCTIONTYPE: '^function \\(', + EXTERNALFUNCTIONTYPE: '^function \\(.*\\).* external', + CONSTANTFUNCTIONTYPE: '^function \\(.*\\).* constant', + REFTYPE: '( storage )|(mapping\\()|(\\[\\])', + FUNCTIONSIGNATURE: '^function \\(([^\\(]*)\\)' } var basicFunctionTypes = { @@ -42,11 +57,175 @@ var specialVariables = { } } +// #################### Trivial Getters + +function getType (node) { + return node.attributes.type +} + +// #################### Complex Getters + +function getFunctionCallType (func) { + if (!(isExternalDirectCall(func) || isThisLocalCall(func) || isLocalCall(func))) throw new Error('staticAnalysisCommon.js: not function call Node') + if (isExternalDirectCall(func)) return func.attributes.type + return findFirstSubNodeLTR(func, exactMatch(nodeTypes.IDENTIFIER)).attributes.type +} + +function getEffectedVariableName (effectNode) { + if (!isEffect(effectNode)) throw new Error('staticAnalysisCommon.js: not an effect Node') + return findFirstSubNodeLTR(effectNode, exactMatch(nodeTypes.IDENTIFIER)).attributes.value +} + +function getLocalCallName (localCallNode) { + if (!isLocalCall(localCallNode)) throw new Error('staticAnalysisCommon.js: not an local call Node') + return localCallNode.children[0].attributes.value +} + +function getThisLocalCallName (localCallNode) { + if (!isThisLocalCall(localCallNode)) throw new Error('staticAnalysisCommon.js: not an this local call Node') + return localCallNode.attributes.value +} + +function getExternalDirectCallContractName (extDirectCall) { + if (!isExternalDirectCall(extDirectCall)) throw new Error('staticAnalysisCommon.js: not an external direct call Node') + return extDirectCall.children[0].attributes.type.replace(new RegExp(basicRegex.CONTRACTTYPE), '') +} + +function getExternalDirectCallMemberName (extDirectCall) { + if (!isExternalDirectCall(extDirectCall)) throw new Error('staticAnalysisCommon.js: not an external direct call Node') + return extDirectCall.attributes.member_name +} + +function getContractName (contract) { + if (!isContractDefinition(contract)) throw new Error('staticAnalysisCommon.js: not an contractDefinition Node') + return contract.attributes.name +} + +function getFunctionDefinitionName (funcDef) { + if (!isFunctionDefinition(funcDef)) throw new Error('staticAnalysisCommon.js: not an functionDefinition Node') + return funcDef.attributes.name +} + +function getInheritsFromName (inheritsNode) { + if (!isInheritanceSpecifier(inheritsNode)) throw new Error('staticAnalysisCommon.js: not an InheritanceSpecifier node Node') + return inheritsNode.children[0].attributes.name +} + +function getDeclaredVariableName (varDeclNode) { + if (!isVariableDeclaration(varDeclNode)) throw new Error('staticAnalysisCommon.js: not an variable declaration') + return varDeclNode.attributes.name +} + +function getStateVariableDeclarationsFormContractNode (contractNode) { + if (!isContractDefinition(contractNode)) throw new Error('staticAnalysisCommon.js: not an contract definition declaration') + return contractNode.children.filter((el) => isVariableDeclaration(el)) +} + +function getFunctionOrModifierDefinitionParameterPart (funcNode) { + if (!isFunctionDefinition(funcNode) && !isModifierDefinition(funcNode)) throw new Error('staticAnalysisCommon.js: not an function definition') + return funcNode.children[0] +} + +function getFunctionCallTypeParameterType (func) { + return new RegExp(basicRegex.FUNCTIONSIGNATURE).exec(getFunctionCallType(func))[1] +} + +function getFullQualifiedFunctionCallIdent (contract, func) { + if (isLocalCall(func)) return getContractName(contract) + '.' + getLocalCallName(func) + '(' + getFunctionCallTypeParameterType(func) + ')' + else if (isThisLocalCall(func)) return getContractName(contract) + '.' + getThisLocalCallName(func) + '(' + getFunctionCallTypeParameterType(func) + ')' + else if (isExternalDirectCall(func)) return getExternalDirectCallContractName(func) + '.' + getExternalDirectCallMemberName(func) + '(' + getFunctionCallTypeParameterType(func) + ')' + else throw new Error('staticAnalysisCommon.js: Can not get function name form non function call node') +} + +function getFullQuallyfiedFuncDefinitionIdent (contract, func, paramTypes) { + return getContractName(contract) + '.' + getFunctionDefinitionName(func) + '(' + utils.concatWithSeperator(paramTypes, ',') + ')' +} + +// #################### Trivial Node Identification + +function isFunctionDefinition (node) { + return nodeType(node, exactMatch(nodeTypes.FUNCTIONDEFINITION)) +} + +function isModifierDefinition (node) { + return nodeType(node, exactMatch(nodeTypes.MODIFIERDEFINITION)) +} + +function isModifierInvocation (node) { + return nodeType(node, exactMatch(nodeTypes.MODIFIERINVOCATION)) +} + +function isVariableDeclaration (node) { + return nodeType(node, exactMatch(nodeTypes.VARIABLEDECLARATION)) +} + +function isInheritanceSpecifier (node) { + return nodeType(node, exactMatch(nodeTypes.INHERITANCESPECIFIER)) +} + +function isAssignment (node) { + return nodeType(node, exactMatch(nodeTypes.ASSIGNMENT)) +} + +function isContractDefinition (node) { + return nodeType(node, exactMatch(nodeTypes.CONTRACTDEFINITION)) +} + +function isInlineAssembly (node) { + return nodeType(node, exactMatch(nodeTypes.INLINEASSEMBLY)) +} + +// #################### Complex Node Identification + +function isStorageVariableDeclaration (node) { + return isVariableDeclaration(node) && expressionType(node, basicRegex.REFTYPE) +} + +function isInteraction (node) { + return isLLCall(node) || isLLSend(node) || isExternalDirectCall(node) +} + +function isEffect (node) { + return isAssignment(node) || isPlusPlusUnaryOperation(node) || isMinusMinusUnaryOperation(node) || isInlineAssembly(node) +} + +function isWriteOnStateVariable (effectNode, stateVariables) { + return isEffect(effectNode) && !isInlineAssembly(effectNode) && isStateVariable(getEffectedVariableName(effectNode), stateVariables) +} + +function isStateVariable (name, stateVariables) { + return stateVariables.some((item) => name === getDeclaredVariableName(item)) +} + +function isConstantFunction (node) { + return isFunctionDefinition(node) && node.attributes.constant === true +} + +function isCallToNonConstLocalFunction (node) { + return isLocalCall(node) && !expressionType(node.children[0], basicRegex.CONSTANTFUNCTIONTYPE) +} + +function isExternalDirectCall (node) { + return isMemberAccess(node, basicRegex.EXTERNALFUNCTIONTYPE, undefined, basicRegex.CONTRACTTYPE, undefined) && !isThisLocalCall(node) +} + // usage of now special variable function isNowAccess (node) { - return nodeType(node, nodeTypes.IDENTIFIER) && - expressionType(node, basicTypes.UINT) && - name(node, 'now') + return nodeType(node, exactMatch(nodeTypes.IDENTIFIER)) && + expressionType(node, exactMatch(basicTypes.UINT)) && + name(node, exactMatch('now')) +} + +function isPlusPlusUnaryOperation (node) { + return nodeType(node, exactMatch(nodeTypes.UNARYOPERATION)) && operator(node, exactMatch(utils.escapeRegExp('++'))) +} + +function isMinusMinusUnaryOperation (node) { + return nodeType(node, exactMatch(nodeTypes.UNARYOPERATION)) && operator(node, exactMatch('--')) +} + +function isFullyImplementedContract (node) { + return nodeType(node, exactMatch(nodeTypes.CONTRACTDEFINITION)) && node.attributes.fullyImplemented === true } // usage of block timestamp @@ -59,7 +238,16 @@ function isSpecialVariableAccess (node, varType) { } function isThisLocalCall (node) { - return isMemberAccess(node, basicRegex.FUNCTIONTYPE, 'this', basicRegex.CONTRACTTYPE, undefined) + return isMemberAccess(node, basicRegex.FUNCTIONTYPE, exactMatch('this'), basicRegex.CONTRACTTYPE, undefined) +} + +function isLocalCall (node) { + return nodeType(node, exactMatch(nodeTypes.FUNCTIONCALL)) && + minNrOfChildren(node, 1) && + nodeType(node.children[0], exactMatch(nodeTypes.IDENTIFIER)) && + expressionType(node.children[0], basicRegex.FUNCTIONTYPE) && + !expressionType(node.children[0], basicRegex.EXTERNALFUNCTIONTYPE) && + nrOfChildren(node.children[0], 0) } function isLowLevelCall (node) { @@ -71,30 +259,30 @@ function isLowLevelCall (node) { function isLLSend (node) { return isMemberAccess(node, - utils.escapeRegExp(lowLevelCallTypes.SEND.type), - undefined, basicTypes.ADDRESS, lowLevelCallTypes.SEND.ident) + exactMatch(utils.escapeRegExp(lowLevelCallTypes.SEND.type)), + undefined, exactMatch(basicTypes.ADDRESS), exactMatch(lowLevelCallTypes.SEND.ident)) } function isLLCall (node) { return isMemberAccess(node, - utils.escapeRegExp(lowLevelCallTypes.CALL.type), - undefined, basicTypes.ADDRESS, lowLevelCallTypes.CALL.ident) + exactMatch(utils.escapeRegExp(lowLevelCallTypes.CALL.type)), + undefined, exactMatch(basicTypes.ADDRESS), exactMatch(lowLevelCallTypes.CALL.ident)) } function isLLCallcode (node) { return isMemberAccess(node, - utils.escapeRegExp(lowLevelCallTypes.CALLCODE.type), - undefined, basicTypes.ADDRESS, lowLevelCallTypes.CALLCODE.ident) + exactMatch(utils.escapeRegExp(lowLevelCallTypes.CALLCODE.type)), + undefined, exactMatch(basicTypes.ADDRESS), exactMatch(lowLevelCallTypes.CALLCODE.ident)) } function isLLDelegatecall (node) { return isMemberAccess(node, - utils.escapeRegExp(lowLevelCallTypes.DELEGATECALL.type), - undefined, basicTypes.ADDRESS, lowLevelCallTypes.DELEGATECALL.ident) + exactMatch(utils.escapeRegExp(lowLevelCallTypes.DELEGATECALL.type)), + undefined, exactMatch(basicTypes.ADDRESS), exactMatch(lowLevelCallTypes.DELEGATECALL.ident)) } function isMemberAccess (node, retType, accessor, accessorType, memberName) { - return nodeType(node, nodeTypes.MEMBERACCESS) && + return nodeType(node, exactMatch(nodeTypes.MEMBERACCESS)) && expressionType(node, retType) && name(node, memberName) && nrOfChildren(node, 1) && @@ -102,8 +290,14 @@ function isMemberAccess (node, retType, accessor, accessorType, memberName) { expressionType(node.children[0], accessorType) } +// #################### Node Identification Primitives + function nrOfChildren (node, nr) { - return (node && (nr === undefined || nr === null)) || (node && node.children && node.children.length === nr) + return (node && (nr === undefined || nr === null)) || (node && nr === 0 && !node.children) || (node && node.children && node.children.length === nr) +} + +function minNrOfChildren (node, nr) { + return (node && (nr === undefined || nr === null)) || (node && nr === 0 && !node.children) || (node && node.children && node.children.length >= nr) } function expressionType (node, typeRegex) { @@ -119,6 +313,16 @@ function name (node, nameRegex) { return (node && !nameRegex) || (node && node.attributes && (regex.test(node.attributes.value) || regex.test(node.attributes.member_name))) } +function operator (node, opRegex) { + return (node && !opRegex) || (node && new RegExp(opRegex).test(node.attributes.operator)) +} + +// #################### Helpers + +function exactMatch (regexStr) { + return '^' + regexStr + '$' +} + /** * Builds an function signature as used in the AST of the solc-json AST * @param {Array} paramTypes @@ -132,15 +336,69 @@ function buildFunctionSignature (paramTypes, returnTypes, isPayable) { return 'function (' + utils.concatWithSeperator(paramTypes, ',') + ')' + ((isPayable) ? ' payable' : '') + ((returnTypes.length) ? ' returns (' + utils.concatWithSeperator(returnTypes, ',') + ')' : '') } +function findFirstSubNodeLTR (node, type) { + for (let i = 0; i < node.children.length; ++i) { + var item = node.children[i] + if (nodeType(item, type)) return item + else { + var res = findFirstSubNodeLTR(item, type) + if (res) return res + } + } + return null +} + module.exports = { + // #################### Trivial Getters + getType: getType, + // #################### Complex Getters + getThisLocalCallName: getThisLocalCallName, + getFunctionCallType: getFunctionCallType, + getContractName: getContractName, + getEffectedVariableName: getEffectedVariableName, + getDeclaredVariableName: getDeclaredVariableName, + getLocalCallName: getLocalCallName, + getInheritsFromName: getInheritsFromName, + getExternalDirectCallContractName: getExternalDirectCallContractName, + getExternalDirectCallMemberName: getExternalDirectCallMemberName, + getFunctionDefinitionName: getFunctionDefinitionName, + getFunctionCallTypeParameterType: getFunctionCallTypeParameterType, + getFullQualifiedFunctionCallIdent: getFullQualifiedFunctionCallIdent, + getFullQuallyfiedFuncDefinitionIdent: getFullQuallyfiedFuncDefinitionIdent, + getStateVariableDeclarationsFormContractNode: getStateVariableDeclarationsFormContractNode, + getFunctionOrModifierDefinitionParameterPart: getFunctionOrModifierDefinitionParameterPart, + + // #################### Complex Node Identification + isInteraction: isInteraction, + isEffect: isEffect, isNowAccess: isNowAccess, isBlockTimestampAccess: isBlockTimestampAccess, isThisLocalCall: isThisLocalCall, + isLocalCall: isLocalCall, + isWriteOnStateVariable: isWriteOnStateVariable, + isStateVariable: isStateVariable, isLowLevelCall: isLowLevelCall, isLowLevelCallInst: isLLCall, isLowLevelCallcodeInst: isLLCallcode, isLowLevelDelegatecallInst: isLLDelegatecall, isLowLevelSendInst: isLLSend, + isExternalDirectCall: isExternalDirectCall, + isFullyImplementedContract: isFullyImplementedContract, + isCallToNonConstLocalFunction: isCallToNonConstLocalFunction, + + // #################### Trivial Node Identification + isFunctionDefinition: isFunctionDefinition, + isModifierDefinition: isModifierDefinition, + isInheritanceSpecifier: isInheritanceSpecifier, + isModifierInvocation: isModifierInvocation, + isVariableDeclaration: isVariableDeclaration, + isStorageVariableDeclaration: isStorageVariableDeclaration, + isAssignment: isAssignment, + isContractDefinition: isContractDefinition, + isConstantFunction: isConstantFunction, + isInlineAssembly: isInlineAssembly, + + // #################### Constants nodeTypes: nodeTypes, basicTypes: basicTypes, basicFunctionTypes: basicFunctionTypes, diff --git a/src/app/staticanalysis/modules/thisLocal.js b/src/app/staticanalysis/modules/thisLocal.js index a0fbb316fe..9369a84c0a 100644 --- a/src/app/staticanalysis/modules/thisLocal.js +++ b/src/app/staticanalysis/modules/thisLocal.js @@ -12,7 +12,6 @@ thisLocal.prototype.visit = function (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 functions in the same contract, it only consumes more gas than normal local calls.`, diff --git a/src/app/staticanalysis/staticAnalysisRunner.js b/src/app/staticanalysis/staticAnalysisRunner.js index d6aca04a04..0583fb30d4 100644 --- a/src/app/staticanalysis/staticAnalysisRunner.js +++ b/src/app/staticanalysis/staticAnalysisRunner.js @@ -12,6 +12,10 @@ staticAnalysisRunner.prototype.run = function (compilationResult, toRun, callbac return { 'name': m.name, 'mod': new m.Module() } }) + this.runWithModuleList(compilationResult, modules, callback) +} + +staticAnalysisRunner.prototype.runWithModuleList = function (compilationResult, modules, callback) { // Also provide convenience analysis via the AST walker. var walker = new AstWalker() for (var k in compilationResult.sources) { diff --git a/test/index.js b/test/index.js index 3de953f129..012067fe17 100644 --- a/test/index.js +++ b/test/index.js @@ -5,3 +5,4 @@ require('./gist-handler-test') require('./query-params-test') require('./util-test') require('./staticanalysis/staticAnalysisCommon-test') +require('./staticanalysis/staticAnalysisIntegration-test') diff --git a/test/staticanalysis/staticAnalysisCommon-test.js b/test/staticanalysis/staticAnalysisCommon-test.js index b815dce6c4..e6f458141d 100644 --- a/test/staticanalysis/staticAnalysisCommon-test.js +++ b/test/staticanalysis/staticAnalysisCommon-test.js @@ -80,7 +80,7 @@ test('staticAnalysisCommon.helpers.nrOfChildren', function (t) { 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') + t.ok(common.helpers.nrOfChildren(node3, 0), 'should work without children arr') lowlevelAccessersCommon(t, common.helpers.nrOfChildren, node) }) @@ -94,8 +94,8 @@ function lowlevelAccessersCommon (t, f, someNode) { t.notOk(f(), 'false on no params') } -test('staticAnalysisCommon.helpers.isLowLevelCall', function (t) { - t.plan(4) +test('staticAnalysisCommon.isLowLevelCall', function (t) { + t.plan(6) 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)' } } @@ -103,11 +103,13 @@ test('staticAnalysisCommon.helpers.isLowLevelCall', function (t) { 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.notOk(common.isLowLevelCallInst(callcodeAst), 'callcode is not call') t.ok(common.isLowLevelCallcodeInst(callcodeAst) && common.isLowLevelCall(callcodeAst), 'callcode is llc should work') + t.notOk(common.isLowLevelCallcodeInst(callAst), 'call is not callcode') t.ok(common.isLowLevelDelegatecallInst(delegatecallAst) && common.isLowLevelCall(delegatecallAst), 'delegatecall is llc should work') }) -test('staticAnalysisCommon.helpers.isThisLocalCall', function (t) { +test('staticAnalysisCommon.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') @@ -115,7 +117,7 @@ test('staticAnalysisCommon.helpers.isThisLocalCall', function (t) { t.notOk(common.isNowAccess(node), 'is now used should not work') }) -test('staticAnalysisCommon.helpers.isBlockTimestampAccess', function (t) { +test('staticAnalysisCommon.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') @@ -123,10 +125,41 @@ test('staticAnalysisCommon.helpers.isBlockTimestampAccess', function (t) { t.notOk(common.isNowAccess(node), 'is now used should not work') }) -test('staticAnalysisCommon.helpers.isNowAccess', function (t) { +test('staticAnalysisCommon.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') }) + +test('staticAnalysisCommon.isExternalDirectCall', function (t) { + t.plan(5) + var node = { + attributes: { + member_name: 'info', + type: 'function () payable external returns (uint256)' + }, + children: [ + { + attributes: { + type: 'contract InfoFeed', + value: 'f' + }, + id: 30, + name: 'Identifier', + src: '405:1:0' + } + ], + id: 32, + name: 'MemberAccess', + src: '405:6:0' + } + + var node2 = { name: 'MemberAccess', children: [{attributes: { value: 'this', type: 'contract test' }}], attributes: { value: 'b', type: 'function (bytes32,address) returns (bool)' } } + 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.notOk(common.isNowAccess(node), 'is now used should not work') + t.ok(common.isExternalDirectCall(node), 'f.info() should be external direct call') + t.notOk(common.isExternalDirectCall(node2), 'local call is not an exernal call') +}) diff --git a/test/staticanalysis/staticAnalysisIntegration-test.js b/test/staticanalysis/staticAnalysisIntegration-test.js new file mode 100644 index 0000000000..d784bddc67 --- /dev/null +++ b/test/staticanalysis/staticAnalysisIntegration-test.js @@ -0,0 +1,47 @@ +// var test = require('tape') + +// var common = require('../../babelify-src/app/staticanalysis/modules/staticAnalysisCommon') +// var StatRunner = require('../../babelify-src/app/staticanalysis/staticAnalysisRunner') +// var utils = require('../../babelify-src/app/utils') +// var Compiler = require('../../babelify-src/app/compiler') + +// var fs = require('fs') +// var path = require('path') + +// var testFiles = [ +// '/test-contracts/KingOfTheEtherThrone.sol', +// '/test-contracts/assembly.sol', +// '/test-contracts/ballot.sol', +// '/test-contracts/ballot_reentrant.sol', +// '/test-contracts/ballot_withoutWarning.sol', +// '/test-contracts/cross_contract.sol', +// '/test-contracts/inheritance.sol', +// '/test-contracts/notReentrant.sol', +// '/test-contracts/structReentrant.sol', +// '/test-contracts/thisLocal.sol', +// '/test-contracts/modifier1.sol', +// '/test-contracts/modifier2.sol' +// ] + +// test('thisLocal.js', function (t) { +// t.plan(0) + +// var module = require('../../babelify-src/app/staticanalysis/modules/thisLocal') + +// runModuleOnFiles(module, t) +// }) + +// function runModuleOnFiles (module, t) { +// var fakeImport = function (url, cb) { cb('Not implemented') } +// var compiler = new Compiler(fakeImport) +// var statRunner = new StatRunner() + +// testFiles.map((fileName) => { +// var contents = fs.readFileSync(path.join(__dirname, fileName), 'utf8') +// var compres = compiler.compile({ 'test': contents }, 'test') + +// statRunner.runWithModuleList(compres, [{ name: module.name, mod: new module.Module() }], (reports) => { +// reports.map((r) => t.comment(r.warning)) +// }) +// }) +// } diff --git a/test/staticanalysis/test-contracts/KingOfTheEtherThrone.sol b/test/staticanalysis/test-contracts/KingOfTheEtherThrone.sol new file mode 100644 index 0000000000..70cfb08327 --- /dev/null +++ b/test/staticanalysis/test-contracts/KingOfTheEtherThrone.sol @@ -0,0 +1,23 @@ +// return value send +contract KingOfTheEtherThrone{ + struct Monarch { + // address of the king . + address ethAddr ; + string name ; + // how much he pays to previous king + uint claimPrice ; + uint coronationTimestamp; + } + Monarch public currentMonarch ; + + function claimThrone ( string name ) { + address wizardAddress; + uint compensation = 100; + uint valuePaid = 10; + + if ( currentMonarch.ethAddr != wizardAddress ) + if (currentMonarch.ethAddr.send( compensation )) throw; + + currentMonarch = Monarch(msg.sender,name,valuePaid,block.timestamp); + } +} diff --git a/test/staticanalysis/test-contracts/assembly.sol b/test/staticanalysis/test-contracts/assembly.sol new file mode 100644 index 0000000000..d1f63e75e1 --- /dev/null +++ b/test/staticanalysis/test-contracts/assembly.sol @@ -0,0 +1,26 @@ +pragma solidity ^0.4.9; + contract test { + + function at(address _addr) returns (bytes o_code) { + assembly { + // retrieve the size of the code, this needs assembly + let size := extcodesize(_addr) + // allocate output byte array - this could also be done without assembly + // by using o_code = new bytes(size) + o_code := mload(0x40) + // new "memory end" including padding + mstore(0x40, add(o_code, and(add(add(size, 0x20), 0x1f), not(0x1f)))) + // store length in memory + mstore(o_code, size) + // actually retrieve the code, this needs assembly + extcodecopy(_addr, add(o_code, 0x20), 0, size) + } + } + + function bla() { + msg.sender.send(19); + assembly { + + } + } + } diff --git a/test/staticanalysis/test-contracts/ballot.sol b/test/staticanalysis/test-contracts/ballot.sol new file mode 100644 index 0000000000..e1e9f676fe --- /dev/null +++ b/test/staticanalysis/test-contracts/ballot.sol @@ -0,0 +1,145 @@ +pragma solidity ^0.4.0; + +/// @title Voting with delegation. +contract Ballot { + // This declares a new complex type which will + // be used for variables later. + // It will represent a single voter. + struct Voter { + uint weight; // weight is accumulated by delegation + bool voted; // if true, that person already voted + address delegate; // person delegated to + uint vote; // index of the voted proposal + } + + // This is a type for a single proposal. + struct Proposal + { + bytes32 name; // short name (up to 32 bytes) + uint voteCount; // number of accumulated votes + } + + address public chairperson; + + // This declares a state variable that + // stores a `Voter` struct for each possible address. + mapping(address => Voter) public voters; + + // A dynamically-sized array of `Proposal` structs. + Proposal[] public proposals; + + /// Create a new ballot to choose one of `proposalNames`. + function Ballot(bytes32[] proposalNames) { + chairperson = msg.sender; + voters[chairperson].weight = 1; + + // For each of the provided proposal names, + // create a new proposal object and add it + // to the end of the array. + for (uint i = 0; i < proposalNames.length; i++) { + // `Proposal({...})` creates a temporary + // Proposal object and `proposals.push(...)` + // appends it to the end of `proposals`. + proposals.push(Proposal({ + name: proposalNames[i], + voteCount: 0 + })); + } + } + + // Give `voter` the right to vote on this ballot. + // May only be called by `chairperson`. + function giveRightToVote(address voter) { + if (msg.sender != chairperson || voters[voter].voted) { + // `throw` terminates and reverts all changes to + // the state and to Ether balances. It is often + // a good idea to use this if functions are + // called incorrectly. But watch out, this + // will also consume all provided gas. + throw; + } + voters[voter].weight = 1; + } + + /// Delegate your vote to the voter `to`. + function delegate(address to) { + // assigns reference + Voter sender = voters[msg.sender]; + if (sender.voted) + throw; + + // Forward the delegation as long as + // `to` also delegated. + // In general, such loops are very dangerous, + // because if they run too long, they might + // need more gas than is available in a block. + // In this case, the delegation will not be executed, + // but in other situations, such loops might + // cause a contract to get "stuck" completely. + while ( + voters[to].delegate != address(0) && + voters[to].delegate != msg.sender + ) { + to = voters[to].delegate; + } + + // We found a loop in the delegation, not allowed. + if (to == msg.sender) { + throw; + } + + // Since `sender` is a reference, this + // modifies `voters[msg.sender].voted` + sender.voted = true; + sender.delegate = to; + Voter delegate = voters[to]; + if (delegate.voted) { + // If the delegate already voted, + // directly add to the number of votes + proposals[delegate.vote].voteCount += sender.weight; + } else { + // If the delegate did not vote yet, + // add to her weight. + delegate.weight += sender.weight; + } + } + + /// Give your vote (including votes delegated to you) + /// to proposal `proposals[proposal].name`. + function vote(uint proposal) { + Voter sender = voters[msg.sender]; + if (sender.voted) + throw; + sender.voted = true; + sender.vote = proposal; + + // If `proposal` is out of the range of the array, + // this will throw automatically and revert all + // changes. + proposals[proposal].voteCount += sender.weight; + } + + /// @dev Computes the winning proposal taking all + /// previous votes into account. + function winningProposal() constant + returns (uint winningProposal) + { + uint winningVoteCount = 0; + for (uint p = 0; p < proposals.length; p++) { + if (proposals[p].voteCount > winningVoteCount) { + winningVoteCount = proposals[p].voteCount; + winningProposal = p; + } + } + } + + // Calls winningProposal() function to get the index + // of the winner contained in the proposals array and then + // returns the name of the winner + function winnerName() constant + returns (bytes32 winnerName) + { + winnerName = proposals[winningProposal()].name; + } +} + diff --git a/test/staticanalysis/test-contracts/ballot_reentrant.sol b/test/staticanalysis/test-contracts/ballot_reentrant.sol new file mode 100644 index 0000000000..a695b09e19 --- /dev/null +++ b/test/staticanalysis/test-contracts/ballot_reentrant.sol @@ -0,0 +1,101 @@ +pragma solidity ^0.4.0; + +contract InfoFeed { + function info() payable returns (uint ret); + function call1(uint a) payable returns (bool); +} + + +contract Ballot { + + InfoFeed feed; + + struct Voter { + uint weight; + bool voted; + uint8 vote; + address delegate; + } + struct Proposal { + uint voteCount; + } + + address chairperson; + mapping(address => Voter) voters; + Proposal[] proposals; + + function send1(address a) { + giveRightToVote(a,a); + } + + /// Create a new ballot with $(_numProposals) different proposals. + function Ballot(uint8 _numProposals) { + address d; + if(!d.delegatecall.gas(800)('function_name', 'arg1', 'arg2')) throw; + if(!d.callcode.gas(800)('function_name', 'arg1', 'arg2')) throw; + if(!d.call.value(10).gas(800)('function_name', 'arg1', 'arg2')) throw; + if(!d.call.value(10).gas(800)('function_name', 'arg1', 'arg2')) throw; + + + + if(!msg.sender.send(1 wei)) throw; + if(!d.call('function_name', 'arg1', 'arg2')) throw; + + + uint a = now; + uint c = block.timestamp; + if(block.timestamp < 100){} + chairperson = msg.sender; + voters[chairperson].weight = 1; + proposals.length = _numProposals; + if(!d.send(1 wei)) throw; + feed.info.value(10).gas(800)(); + + feed.call1(1); + + this.send1(d); + } + + + /// Give $(voter) the right to vote on this ballot. + /// May only be called by $(chairperson). + function giveRightToVote(address voter, address b) payable returns (bool){ + if (msg.sender != chairperson || voters[voter].voted) return; + voters[voter].weight = 1; + return true; + } + + /// Delegate your vote to the voter $(to). + function delegate(address to) { + Voter sender = voters[msg.sender]; // assigns reference + if (sender.voted) return; + while (voters[to].delegate != address(0) && voters[to].delegate != msg.sender) + to = voters[to].delegate; + if (to == msg.sender) return; + sender.voted = true; + sender.delegate = to; + Voter delegate = voters[to]; + if (delegate.voted) + proposals[delegate.vote].voteCount += sender.weight; + else + delegate.weight += sender.weight; + } + + /// Give a single vote to proposal $(proposal). + function vote(uint8 proposal) { + Voter sender = voters[msg.sender]; + if (sender.voted || proposal >= proposals.length) return; + sender.voted = true; + sender.vote = proposal; + proposals[proposal].voteCount += sender.weight; + } + + function winningProposal() constant returns (uint8 winningProposal) { + uint256 winningVoteCount = 0; + for (uint8 proposal = 0; proposal < proposals.length; proposal++) + if (proposals[proposal].voteCount > winningVoteCount) { + winningVoteCount = proposals[proposal].voteCount; + winningProposal = proposal; + } + } +} diff --git a/test/staticanalysis/test-contracts/ballot_withoutWarnings.sol b/test/staticanalysis/test-contracts/ballot_withoutWarnings.sol new file mode 100644 index 0000000000..e273b3da4f --- /dev/null +++ b/test/staticanalysis/test-contracts/ballot_withoutWarnings.sol @@ -0,0 +1,31 @@ +pragma solidity ^0.4.0; + +/// @title Voting with delegation. +contract Ballot { + + struct Proposal + { + bytes32 name; // short name (up to 32 bytes) + uint voteCount; // number of accumulated votes + } + + // A dynamically-sized array of `Proposal` structs. + Proposal[] public proposals; + + /// @dev Computes the winning proposal taking all + /// previous votes into account. + function winningProposal() constant + returns (uint winningProposal) + { + winningProposal = 0; + } + + // Calls winningProposal() function to get the index + // of the winner contained in the proposals array and then + // returns the name of the winner + function winnerName() constant + returns (bytes32 winnerName) + { + winnerName = proposals[winningProposal()].name; + } +} diff --git a/test/staticanalysis/test-contracts/cross_contract.sol b/test/staticanalysis/test-contracts/cross_contract.sol new file mode 100644 index 0000000000..426c8d444d --- /dev/null +++ b/test/staticanalysis/test-contracts/cross_contract.sol @@ -0,0 +1,19 @@ +pragma solidity ^0.4.0; + +contract a { + + uint x; + + function foo() { + x++; + } +} + +contract b { + a x; + function bar() constant { + address a; + a.send(100 wei); + x.foo(); + } +} diff --git a/test/staticanalysis/test-contracts/inheritance.sol b/test/staticanalysis/test-contracts/inheritance.sol new file mode 100644 index 0000000000..1491e8fa9f --- /dev/null +++ b/test/staticanalysis/test-contracts/inheritance.sol @@ -0,0 +1,40 @@ +pragma solidity ^0.4.9; + +contract r { + function s() constant {} +} + +contract a is r { + uint x = 1; + + function getX() constant returns (uint) { + return x; + } +} + +contract b is a { + uint y = 2; + uint x = 3; + + + function getY(uint z, bool r) returns (uint) { + return y++; + } + + function getY(string storage n) internal constant returns (uint) { return 10; } + +} + +contract c is b { + string x; + + function d() returns (uint a, uint b) { + //d(); + //sha3("bla"); + msg.sender.call.gas(200000).value(this.balance)(bytes4(sha3("pay()"))); + //x++; + getY(x); + a = getX() + getY(1, false); + b = getX() + getY(x); + } +} diff --git a/test/staticanalysis/test-contracts/modifier1.sol b/test/staticanalysis/test-contracts/modifier1.sol new file mode 100644 index 0000000000..a755d763b1 --- /dev/null +++ b/test/staticanalysis/test-contracts/modifier1.sol @@ -0,0 +1,17 @@ +pragma solidity ^0.4.0; + +contract test { + + address owner; + + modifier onlyOwner { + var a = 0; + if (msg.sender != owner) + throw; + _; + } + + function b(address a) onlyOwner returns (bool) { + + } +} \ No newline at end of file diff --git a/test/staticanalysis/test-contracts/modifier2.sol b/test/staticanalysis/test-contracts/modifier2.sol new file mode 100644 index 0000000000..44db1617c7 --- /dev/null +++ b/test/staticanalysis/test-contracts/modifier2.sol @@ -0,0 +1,28 @@ +pragma solidity ^0.4.0; + +contract owned { + + uint r=0; + + modifier ntimes(uint n) { + for(uint i=0;i uint) shares; + /// Withdraw your share. + function withdraw() { + var share = shares[msg.sender]; + shares[msg.sender] = 0; + if (!msg.sender.send(share)) + throw; + } +} diff --git a/test/staticanalysis/test-contracts/reentrant.sol b/test/staticanalysis/test-contracts/reentrant.sol new file mode 100644 index 0000000000..896395e1a6 --- /dev/null +++ b/test/staticanalysis/test-contracts/reentrant.sol @@ -0,0 +1,49 @@ +pragma solidity ^0.4.0; + +contract InfoFeed { + uint c; + function info() constant returns (uint ret) {return c;} + function call1(uint a) constant returns (bool) {return true;} +} + +// THIS CONTRACT CONTAINS A BUG - DO NOT USE +contract Fund { + /// Mapping of ether shares of the contract. + //mapping(address => uint) shares; + /// Withdraw your share. + + uint c = 0; + function withdraw() constant { + InfoFeed f; + + + //shares[msg.sender] /= 1; + + f.info(); + + //if (msg.sender.send(shares[msg.sender])) throw; + // shares[msg.sender] = 0; + + + b(true, false); + //shares[msg.sender]++; + //c++; + + } + mapping(address => uint) shares; + + function b(bool a, bool b) returns (bool) { + mapping(address => uint) c = shares; + c[msg.sender] = 0; + //f(); + //withdraw(); + //shares[msg.sender]++; + //c++; + return true; + } + + function f() { + c++; + withdraw(); + } +} diff --git a/test/staticanalysis/test-contracts/structReentrant.sol b/test/staticanalysis/test-contracts/structReentrant.sol new file mode 100644 index 0000000000..95952ff1ca --- /dev/null +++ b/test/staticanalysis/test-contracts/structReentrant.sol @@ -0,0 +1,36 @@ +pragma solidity ^0.4.9; + +contract Ballot { + + struct Voter { + uint weight; + bool voted; + uint8 vote; + address delegate; + baz foo; + } + + struct baz{ + uint bar; + } + + mapping(address => Voter) voters; + + /// Create a new ballot with $(_numProposals) different proposals. + function bla(address a) { + Voter x = voters[a]; + + if (!a.send(10)) + throw; + + //voters[a] = Voter(10,true,1,a); + //x.foo.bar *= 100; + bli(x); + } + + //function bla(){} + + function bli(Voter storage x) private { + x.foo.bar++; + } +} diff --git a/test/staticanalysis/test-contracts/thisLocal.sol b/test/staticanalysis/test-contracts/thisLocal.sol new file mode 100644 index 0000000000..e31cf0dbaf --- /dev/null +++ b/test/staticanalysis/test-contracts/thisLocal.sol @@ -0,0 +1,16 @@ +pragma solidity ^0.4.0; + +contract test { + + function (){ + address x; + this.b(x); + x.call('something'); + x.send(1 wei); + + } + + function b(address a) returns (bool) { + + } +} From d68814df4b1a79334863c1db4d7576061ae86616 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michael=20Fr=C3=B6wis?= Date: Fri, 31 Mar 2017 10:07:11 +0200 Subject: [PATCH 02/18] Static Analysis: more unit tests --- .../modules/staticAnalysisCommon.js | 51 +- .../staticAnalysisCommon-test.js | 1563 ++++++++++++++++- 2 files changed, 1559 insertions(+), 55 deletions(-) diff --git a/src/app/staticanalysis/modules/staticAnalysisCommon.js b/src/app/staticanalysis/modules/staticAnalysisCommon.js index c0f94960e5..8c4190f840 100644 --- a/src/app/staticanalysis/modules/staticAnalysisCommon.js +++ b/src/app/staticanalysis/modules/staticAnalysisCommon.js @@ -67,12 +67,12 @@ function getType (node) { function getFunctionCallType (func) { if (!(isExternalDirectCall(func) || isThisLocalCall(func) || isLocalCall(func))) throw new Error('staticAnalysisCommon.js: not function call Node') - if (isExternalDirectCall(func)) return func.attributes.type + if (isExternalDirectCall(func) || isThisLocalCall(func)) return func.attributes.type return findFirstSubNodeLTR(func, exactMatch(nodeTypes.IDENTIFIER)).attributes.type } function getEffectedVariableName (effectNode) { - if (!isEffect(effectNode)) throw new Error('staticAnalysisCommon.js: not an effect Node') + if (!isEffect(effectNode) || isInlineAssembly(effectNode)) throw new Error('staticAnalysisCommon.js: not an effect Node or inline assembly') return findFirstSubNodeLTR(effectNode, exactMatch(nodeTypes.IDENTIFIER)).attributes.value } @@ -91,6 +91,11 @@ function getExternalDirectCallContractName (extDirectCall) { return extDirectCall.children[0].attributes.type.replace(new RegExp(basicRegex.CONTRACTTYPE), '') } +function getThisLocalCallContractName (thisLocalCall) { + if (!isThisLocalCall(thisLocalCall)) throw new Error('staticAnalysisCommon.js: not an this local call Node') + return thisLocalCall.children[0].attributes.type.replace(new RegExp(basicRegex.CONTRACTTYPE), '') +} + function getExternalDirectCallMemberName (extDirectCall) { if (!isExternalDirectCall(extDirectCall)) throw new Error('staticAnalysisCommon.js: not an external direct call Node') return extDirectCall.attributes.member_name @@ -132,7 +137,7 @@ function getFunctionCallTypeParameterType (func) { function getFullQualifiedFunctionCallIdent (contract, func) { if (isLocalCall(func)) return getContractName(contract) + '.' + getLocalCallName(func) + '(' + getFunctionCallTypeParameterType(func) + ')' - else if (isThisLocalCall(func)) return getContractName(contract) + '.' + getThisLocalCallName(func) + '(' + getFunctionCallTypeParameterType(func) + ')' + else if (isThisLocalCall(func)) return getThisLocalCallContractName(func) + '.' + getThisLocalCallName(func) + '(' + getFunctionCallTypeParameterType(func) + ')' else if (isExternalDirectCall(func)) return getExternalDirectCallContractName(func) + '.' + getExternalDirectCallMemberName(func) + '(' + getFunctionCallTypeParameterType(func) + ')' else throw new Error('staticAnalysisCommon.js: Can not get function name form non function call node') } @@ -201,6 +206,18 @@ function isConstantFunction (node) { return isFunctionDefinition(node) && node.attributes.constant === true } +function isPlusPlusUnaryOperation (node) { + return nodeType(node, exactMatch(nodeTypes.UNARYOPERATION)) && operator(node, exactMatch(utils.escapeRegExp('++'))) +} + +function isMinusMinusUnaryOperation (node) { + return nodeType(node, exactMatch(nodeTypes.UNARYOPERATION)) && operator(node, exactMatch(utils.escapeRegExp('--'))) +} + +function isFullyImplementedContract (node) { + return nodeType(node, exactMatch(nodeTypes.CONTRACTDEFINITION)) && node.attributes.fullyImplemented === true +} + function isCallToNonConstLocalFunction (node) { return isLocalCall(node) && !expressionType(node.children[0], basicRegex.CONSTANTFUNCTIONTYPE) } @@ -216,27 +233,11 @@ function isNowAccess (node) { name(node, exactMatch('now')) } -function isPlusPlusUnaryOperation (node) { - return nodeType(node, exactMatch(nodeTypes.UNARYOPERATION)) && operator(node, exactMatch(utils.escapeRegExp('++'))) -} - -function isMinusMinusUnaryOperation (node) { - return nodeType(node, exactMatch(nodeTypes.UNARYOPERATION)) && operator(node, exactMatch('--')) -} - -function isFullyImplementedContract (node) { - return nodeType(node, exactMatch(nodeTypes.CONTRACTDEFINITION)) && node.attributes.fullyImplemented === true -} - // usage of block timestamp function isBlockTimestampAccess (node) { return isSpecialVariableAccess(node, specialVariables.BLOCKTIMESTAMP) } -function isSpecialVariableAccess (node, varType) { - return isMemberAccess(node, varType.type, varType.obj, varType.obj, varType.member) -} - function isThisLocalCall (node) { return isMemberAccess(node, basicRegex.FUNCTIONTYPE, exactMatch('this'), basicRegex.CONTRACTTYPE, undefined) } @@ -281,6 +282,8 @@ function isLLDelegatecall (node) { undefined, exactMatch(basicTypes.ADDRESS), exactMatch(lowLevelCallTypes.DELEGATECALL.ident)) } +// #################### Complex Node Identification - Private + function isMemberAccess (node, retType, accessor, accessorType, memberName) { return nodeType(node, exactMatch(nodeTypes.MEMBERACCESS)) && expressionType(node, retType) && @@ -290,6 +293,10 @@ function isMemberAccess (node, retType, accessor, accessorType, memberName) { expressionType(node.children[0], accessorType) } +function isSpecialVariableAccess (node, varType) { + return isMemberAccess(node, varType.type, varType.obj, varType.obj, varType.member) +} + // #################### Node Identification Primitives function nrOfChildren (node, nr) { @@ -337,6 +344,7 @@ function buildFunctionSignature (paramTypes, returnTypes, isPayable) { } function findFirstSubNodeLTR (node, type) { + if (!node || !node.children) return null for (let i = 0; i < node.children.length; ++i) { var item = node.children[i] if (nodeType(item, type)) return item @@ -360,6 +368,7 @@ module.exports = { getLocalCallName: getLocalCallName, getInheritsFromName: getInheritsFromName, getExternalDirectCallContractName: getExternalDirectCallContractName, + getThisLocalCallContractName: getThisLocalCallContractName, getExternalDirectCallMemberName: getExternalDirectCallMemberName, getFunctionDefinitionName: getFunctionDefinitionName, getFunctionCallTypeParameterType: getFunctionCallTypeParameterType, @@ -385,6 +394,8 @@ module.exports = { isExternalDirectCall: isExternalDirectCall, isFullyImplementedContract: isFullyImplementedContract, isCallToNonConstLocalFunction: isCallToNonConstLocalFunction, + isPlusPlusUnaryOperation: isPlusPlusUnaryOperation, + isMinusMinusUnaryOperation: isMinusMinusUnaryOperation, // #################### Trivial Node Identification isFunctionDefinition: isFunctionDefinition, @@ -406,9 +417,11 @@ module.exports = { specialVariables: specialVariables, helpers: { nrOfChildren: nrOfChildren, + minNrOfChildren: minNrOfChildren, expressionType: expressionType, nodeType: nodeType, name: name, + operator: operator, buildFunctionSignature: buildFunctionSignature } } diff --git a/test/staticanalysis/staticAnalysisCommon-test.js b/test/staticanalysis/staticAnalysisCommon-test.js index e6f458141d..9d1e85ea2d 100644 --- a/test/staticanalysis/staticAnalysisCommon-test.js +++ b/test/staticanalysis/staticAnalysisCommon-test.js @@ -3,6 +3,8 @@ var test = require('tape') var common = require('../../src/app/staticanalysis/modules/staticAnalysisCommon') var utils = require('../../src/app/utils') +// #################### helpers Test + test('staticAnalysisCommon.helpers.buildFunctionSignature', function (t) { t.plan(7) @@ -35,6 +37,8 @@ test('staticAnalysisCommon.helpers.buildFunctionSignature', function (t) { 'check fixed call type') }) +// #################### Node Identification Primitives + test('staticAnalysisCommon.helpers.name', function (t) { t.plan(9) var node = { attributes: { value: 'now' } } @@ -47,6 +51,22 @@ test('staticAnalysisCommon.helpers.name', function (t) { lowlevelAccessersCommon(t, common.helpers.name, node) }) +test('staticAnalysisCommon.helpers.operator', function (t) { + t.plan(10) + var node = { attributes: { operator: '++' } } + var node2 = { attributes: { operator: '+++' } } + + var escapedPP = utils.escapeRegExp('++') + var escapedPPExact = `^${escapedPP}$` + + t.ok(common.helpers.operator(node, escapedPPExact), 'should work for ++') + t.notOk(common.helpers.operator(node2, escapedPPExact), 'should not work for +++') + t.ok(common.helpers.operator(node, escapedPP), 'should work for ++') + t.ok(common.helpers.operator(node2, escapedPP), 'should work for +++') + + lowlevelAccessersCommon(t, common.helpers.operator, node) +}) + test('staticAnalysisCommon.helpers.nodeType', function (t) { t.plan(9) var node = { name: 'Identifier', attributes: { name: 'now' } } @@ -85,6 +105,23 @@ test('staticAnalysisCommon.helpers.nrOfChildren', function (t) { lowlevelAccessersCommon(t, common.helpers.nrOfChildren, node) }) +test('staticAnalysisCommon.helpers.minNrOfChildren', function (t) { + t.plan(13) + 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.minNrOfChildren(node, 2), 'should work for 2 children') + t.ok(common.helpers.minNrOfChildren(node, 1), 'should work for 1 children') + t.ok(common.helpers.minNrOfChildren(node, 0), 'should work for 0 children') + t.notOk(common.helpers.minNrOfChildren(node, 3), 'has less than 3 children') + t.notOk(common.helpers.minNrOfChildren(node, '1+2'), 'regex should not work') + t.ok(common.helpers.minNrOfChildren(node2, 0), 'should work for 0 children') + t.ok(common.helpers.minNrOfChildren(node3, 0), 'should work without children arr') + + lowlevelAccessersCommon(t, common.helpers.minNrOfChildren, 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') @@ -94,48 +131,302 @@ function lowlevelAccessersCommon (t, f, someNode) { t.notOk(f(), 'false on no params') } -test('staticAnalysisCommon.isLowLevelCall', function (t) { - t.plan(6) - 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)' } } +// #################### Trivial Getter Test - 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.notOk(common.isLowLevelCallInst(callcodeAst), 'callcode is not call') - t.ok(common.isLowLevelCallcodeInst(callcodeAst) && common.isLowLevelCall(callcodeAst), 'callcode is llc should work') - t.notOk(common.isLowLevelCallcodeInst(callAst), 'call is not callcode') - t.ok(common.isLowLevelDelegatecallInst(delegatecallAst) && common.isLowLevelCall(delegatecallAst), 'delegatecall is llc should work') +test('staticAnalysisCommon.getType', function (t) { + t.plan(3) + 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.getType(node) === 'uint256', 'gettype should work for different nodes') + t.ok(common.getType(node2) === 'function () payable returns (bool)', 'gettype should work for different nodes') + t.ok(common.getType(node3) === 'function () payable returns (bool)', 'gettype should work for different nodes') }) -test('staticAnalysisCommon.isThisLocalCall', function (t) { +// #################### Complex Getter Test + +test('staticAnalysisCommon.getFunctionCallType', function (t) { + t.plan(4) + var localCall = { + 'attributes': { + 'type': 'tuple()', + 'type_conversion': false + }, + 'children': [ + { + 'attributes': { + 'type': 'function (struct Ballot.Voter storage pointer)', + 'value': 'bli' + }, + 'id': 37, + 'name': 'Identifier', + 'src': '540:3:0' + }, + { + 'attributes': { + 'type': 'struct Ballot.Voter storage pointer', + 'value': 'x' + }, + 'id': 38, + 'name': 'Identifier', + 'src': '544:1:0' + } + ], + 'id': 39, + 'name': 'FunctionCall', + 'src': '540:6:0' + } + var thisLocalCall = { name: 'MemberAccess', children: [ { attributes: { value: 'this', type: 'contract test' }, name: 'Identifier' } ], attributes: { value: 'b', type: 'function (bytes32,address) returns (bool)' } } + var externalDirect = { + attributes: { + member_name: 'info', + type: 'function () payable external returns (uint256)' + }, + children: [ + { + attributes: { + type: 'contract InfoFeed', + value: 'f' + }, + id: 30, + name: 'Identifier', + src: '405:1:0' + } + ], + id: 32, + name: 'MemberAccess', + src: '405:6:0' + } + t.ok(common.getFunctionCallType(thisLocalCall) === 'function (bytes32,address) returns (bool)', 'this local call returns correct type') + t.ok(common.getFunctionCallType(externalDirect) === 'function () payable external returns (uint256)', 'external direct call returns correct type') + t.ok(common.getFunctionCallType(localCall) === 'function (struct Ballot.Voter storage pointer)', 'local call returns correct type') + t.throws(() => common.getFunctionCallType({ name: 'MemberAccess' }), undefined, 'throws on wrong type') +}) + +test('staticAnalysisCommon.getEffectedVariableName', 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') + var assignment = { + 'attributes': { + 'operator': '=', + 'type': 'uint256' + }, + 'children': [ + { + 'attributes': { + 'type': 'uint256' + }, + 'children': [ + { + 'attributes': { + 'type': 'mapping(address => uint256)', + 'value': 'c' + }, + 'id': 61, + 'name': 'Identifier', + 'src': '873:1:0' + }, + { + 'attributes': { + 'member_name': 'sender', + 'type': 'address' + }, + 'children': [ + { + 'attributes': { + 'type': 'msg', + 'value': 'msg' + }, + 'id': 62, + 'name': 'Identifier', + 'src': '875:3:0' + } + ], + 'id': 63, + 'name': 'MemberAccess', + 'src': '875:10:0' + } + ], + 'id': 64, + 'name': 'IndexAccess', + 'src': '873:13:0' + }, + { + 'attributes': { + 'hexvalue': '30', + 'subdenomination': null, + 'token': null, + 'type': 'int_const 0', + 'value': '0' + }, + 'id': 65, + 'name': 'Literal', + 'src': '889:1:0' + } + ], + 'id': 66, + 'name': 'Assignment', + 'src': '873:17:0' + } + var inlineAssembly = { + 'children': [ + ], + 'id': 21, + 'name': 'InlineAssembly', + 'src': '809:41:0' + } + t.throws(() => common.getEffectedVariableName(inlineAssembly), 'staticAnalysisCommon.js: not an effect Node or inline assembly', 'get from inline assembly should throw') + t.ok(common.getEffectedVariableName(assignment) === 'c', 'get right name for assignment') + t.throws(() => common.getEffectedVariableName({ name: 'MemberAccess' }), undefined, 'should throw on all other nodes') }) -test('staticAnalysisCommon.isBlockTimestampAccess', function (t) { +test('staticAnalysisCommon.getLocalCallName', 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') + var localCall = { + 'attributes': { + 'type': 'tuple()', + 'type_conversion': false + }, + 'children': [ + { + 'attributes': { + 'type': 'function (struct Ballot.Voter storage pointer)', + 'value': 'bli' + }, + 'id': 37, + 'name': 'Identifier', + 'src': '540:3:0' + }, + { + 'attributes': { + 'type': 'struct Ballot.Voter storage pointer', + 'value': 'x' + }, + 'id': 38, + 'name': 'Identifier', + 'src': '544:1:0' + } + ], + 'id': 39, + 'name': 'FunctionCall', + 'src': '540:6:0' + } + var thisLocalCall = { name: 'MemberAccess', children: [ { attributes: { value: 'this', type: 'contract test' }, name: 'Identifier' } ], attributes: { value: 'b', type: 'function (bytes32,address) returns (bool)' } } + var externalDirect = { + attributes: { + member_name: 'info', + type: 'function () payable external returns (uint256)' + }, + children: [ + { + attributes: { + type: 'contract InfoFeed', + value: 'f' + }, + id: 30, + name: 'Identifier', + src: '405:1:0' + } + ], + id: 32, + name: 'MemberAccess', + src: '405:6:0' + } + t.ok(common.getLocalCallName(localCall) === 'bli', 'getLocal call name from node') + t.throws(() => common.getLocalCallName(externalDirect), undefined, 'throws on other nodes') + t.throws(() => common.getLocalCallName(thisLocalCall), undefined, 'throws on other nodes') }) -test('staticAnalysisCommon.isNowAccess', function (t) { +test('staticAnalysisCommon.getThisLocalCallName', 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') + var localCall = { + 'attributes': { + 'type': 'tuple()', + 'type_conversion': false + }, + 'children': [ + { + 'attributes': { + 'type': 'function (struct Ballot.Voter storage pointer)', + 'value': 'bli' + }, + 'id': 37, + 'name': 'Identifier', + 'src': '540:3:0' + }, + { + 'attributes': { + 'type': 'struct Ballot.Voter storage pointer', + 'value': 'x' + }, + 'id': 38, + 'name': 'Identifier', + 'src': '544:1:0' + } + ], + 'id': 39, + 'name': 'FunctionCall', + 'src': '540:6:0' + } + var thisLocalCall = { name: 'MemberAccess', children: [ { attributes: { value: 'this', type: 'contract test' }, name: 'Identifier' } ], attributes: { value: 'b', type: 'function (bytes32,address) returns (bool)' } } + var externalDirect = { + attributes: { + member_name: 'info', + type: 'function () payable external returns (uint256)' + }, + children: [ + { + attributes: { + type: 'contract InfoFeed', + value: 'f' + }, + id: 30, + name: 'Identifier', + src: '405:1:0' + } + ], + id: 32, + name: 'MemberAccess', + src: '405:6:0' + } + t.ok(common.getThisLocalCallName(thisLocalCall) === 'b', 'get this Local call name from node') + t.throws(() => common.getThisLocalCallName(externalDirect), undefined, 'throws on other nodes') + t.throws(() => common.getThisLocalCallName(localCall), undefined, 'throws on other nodes') }) -test('staticAnalysisCommon.isExternalDirectCall', function (t) { - t.plan(5) - var node = { +test('staticAnalysisCommon.getExternalDirectCallContractName', function (t) { + t.plan(3) + var localCall = { + 'attributes': { + 'type': 'tuple()', + 'type_conversion': false + }, + 'children': [ + { + 'attributes': { + 'type': 'function (struct Ballot.Voter storage pointer)', + 'value': 'bli' + }, + 'id': 37, + 'name': 'Identifier', + 'src': '540:3:0' + }, + { + 'attributes': { + 'type': 'struct Ballot.Voter storage pointer', + 'value': 'x' + }, + 'id': 38, + 'name': 'Identifier', + 'src': '544:1:0' + } + ], + 'id': 39, + 'name': 'FunctionCall', + 'src': '540:6:0' + } + var thisLocalCall = { name: 'MemberAccess', children: [ { attributes: { value: 'this', type: 'contract test' }, name: 'Identifier' } ], attributes: { value: 'b', type: 'function (bytes32,address) returns (bool)' } } + var externalDirect = { attributes: { member_name: 'info', type: 'function () payable external returns (uint256)' @@ -155,11 +446,1211 @@ test('staticAnalysisCommon.isExternalDirectCall', function (t) { name: 'MemberAccess', src: '405:6:0' } + t.ok(common.getExternalDirectCallContractName(externalDirect) === 'InfoFeed', 'external direct call contract name from node') + t.throws(() => common.getExternalDirectCallContractName(thisLocalCall), undefined, 'throws on other nodes') + t.throws(() => common.getExternalDirectCallContractName(localCall), undefined, 'throws on other nodes') +}) - var node2 = { name: 'MemberAccess', children: [{attributes: { value: 'this', type: 'contract test' }}], attributes: { value: 'b', type: 'function (bytes32,address) returns (bool)' } } - 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.notOk(common.isNowAccess(node), 'is now used should not work') - t.ok(common.isExternalDirectCall(node), 'f.info() should be external direct call') - t.notOk(common.isExternalDirectCall(node2), 'local call is not an exernal call') +test('staticAnalysisCommon.getThisLocalCallContractName', function (t) { + t.plan(3) + var localCall = { + 'attributes': { + 'type': 'tuple()', + 'type_conversion': false + }, + 'children': [ + { + 'attributes': { + 'type': 'function (struct Ballot.Voter storage pointer)', + 'value': 'bli' + }, + 'id': 37, + 'name': 'Identifier', + 'src': '540:3:0' + }, + { + 'attributes': { + 'type': 'struct Ballot.Voter storage pointer', + 'value': 'x' + }, + 'id': 38, + 'name': 'Identifier', + 'src': '544:1:0' + } + ], + 'id': 39, + 'name': 'FunctionCall', + 'src': '540:6:0' + } + var thisLocalCall = { name: 'MemberAccess', children: [ { attributes: { value: 'this', type: 'contract test' }, name: 'Identifier' } ], attributes: { value: 'b', type: 'function (bytes32,address) returns (bool)' } } + var externalDirect = { + attributes: { + member_name: 'info', + type: 'function () payable external returns (uint256)' + }, + children: [ + { + attributes: { + type: 'contract InfoFeed', + value: 'f' + }, + id: 30, + name: 'Identifier', + src: '405:1:0' + } + ], + id: 32, + name: 'MemberAccess', + src: '405:6:0' + } + t.ok(common.getThisLocalCallContractName(thisLocalCall) === 'test', 'this local call contract name from node') + t.throws(() => common.getThisLocalCallContractName(localCall), undefined, 'throws on other nodes') + t.throws(() => common.getThisLocalCallContractName(externalDirect), undefined, 'throws on other nodes') +}) + +test('staticAnalysisCommon.getExternalDirectCallMemberName', function (t) { + t.plan(3) + var localCall = { + 'attributes': { + 'type': 'tuple()', + 'type_conversion': false + }, + 'children': [ + { + 'attributes': { + 'type': 'function (struct Ballot.Voter storage pointer)', + 'value': 'bli' + }, + 'id': 37, + 'name': 'Identifier', + 'src': '540:3:0' + }, + { + 'attributes': { + 'type': 'struct Ballot.Voter storage pointer', + 'value': 'x' + }, + 'id': 38, + 'name': 'Identifier', + 'src': '544:1:0' + } + ], + 'id': 39, + 'name': 'FunctionCall', + 'src': '540:6:0' + } + var thisLocalCall = { name: 'MemberAccess', children: [ { attributes: { value: 'this', type: 'contract test' }, name: 'Identifier' } ], attributes: { value: 'b', type: 'function (bytes32,address) returns (bool)' } } + var externalDirect = { + attributes: { + member_name: 'info', + type: 'function () payable external returns (uint256)' + }, + children: [ + { + attributes: { + type: 'contract InfoFeed', + value: 'f' + }, + id: 30, + name: 'Identifier', + src: '405:1:0' + } + ], + id: 32, + name: 'MemberAccess', + src: '405:6:0' + } + t.ok(common.getExternalDirectCallMemberName(externalDirect) === 'info', 'external direct call name from node') + t.throws(() => common.getExternalDirectCallMemberName(thisLocalCall), undefined, 'throws on other nodes') + t.throws(() => common.getExternalDirectCallMemberName(localCall), undefined, 'throws on other nodes') +}) + +test('staticAnalysisCommon.getContractName', function (t) { + t.plan(2) + var contract = { name: 'ContractDefinition', attributes: { name: 'baz' } } + t.ok(common.getContractName(contract) === 'baz', 'returns right contract name') + t.throws(() => common.getContractName({ name: 'InheritanceSpecifier' }), undefined, 'throws on other nodes') +}) + +test('staticAnalysisCommon.getFunctionDefinitionName', function (t) { + t.plan(2) + var func = { name: 'FunctionDefinition', attributes: { name: 'foo' } } + t.ok(common.getFunctionDefinitionName(func) === 'foo', 'returns right contract name') + t.throws(() => common.getFunctionDefinitionName({ name: 'InlineAssembly' }), undefined, 'throws on other nodes') +}) + +test('staticAnalysisCommon.getInheritsFromName', function (t) { + t.plan(2) + var inh = { + 'children': [ + { + 'attributes': { + 'name': 'r' + }, + 'id': 7, + 'name': 'UserDefinedTypeName', + 'src': '84:1:0' + } + ], + 'id': 8, + 'name': 'InheritanceSpecifier', + 'src': '84:1:0' + } + t.ok(common.getInheritsFromName(inh) === 'r', 'returns right contract name') + t.throws(() => common.getInheritsFromName({ name: 'ElementaryTypeName' }), undefined, 'throws on other nodes') +}) + +test('staticAnalysisCommon.getDeclaredVariableName', function (t) { + t.plan(2) + var node1 = { + 'attributes': { + 'name': 'x', + 'type': 'struct Ballot.Voter storage pointer' + }, + 'children': [ + { + 'attributes': { + 'name': 'Voter' + }, + 'id': 43, + 'name': 'UserDefinedTypeName', + 'src': '604:5:0' + } + ], + 'id': 44, + 'name': 'VariableDeclaration', + 'src': '604:15:0' + } + t.ok(common.getDeclaredVariableName(node1) === 'x', 'extract right variable name') + node1.name = 'FunctionCall' + t.throws(() => common.getDeclaredVariableName(node1) === 'x', undefined, 'throw if wrong node') +}) + +test('staticAnalysisCommon.getStateVariableDeclarationsFormContractNode', function (t) { + t.plan(4) + var contract = { + 'attributes': { + 'fullyImplemented': true, + 'isLibrary': false, + 'linearizedBaseContracts': [ + 274 + ], + 'name': 'Ballot' + }, + 'children': [ + { + 'attributes': { + 'name': 'Voter' + }, + 'children': [], + 'name': 'StructDefinition' + }, + { + 'attributes': { + 'name': 'Proposal' + }, + 'children': [], + 'name': 'StructDefinition' + }, + { + 'attributes': { + 'name': 'chairperson', + 'type': 'address' + }, + 'children': [ + { + 'attributes': { + 'name': 'address' + }, + 'name': 'ElementaryTypeName' + } + ], + 'name': 'VariableDeclaration' + }, + { + 'attributes': { + 'name': 'voters', + 'type': 'mapping(address => struct Ballot.Voter storage ref)' + }, + 'children': [ + { + 'children': [ + { + 'attributes': { + 'name': 'address' + }, + 'name': 'ElementaryTypeName' + }, + { + 'attributes': { + 'name': 'Voter' + }, + 'name': 'UserDefinedTypeName' + } + ], + 'name': 'Mapping' + } + ], + 'name': 'VariableDeclaration' + }, + { + 'attributes': { + 'name': 'proposals', + 'type': 'struct Ballot.Proposal storage ref[] storage ref' + }, + 'children': [ + { + 'children': [ + { + 'attributes': { + 'name': 'Proposal' + }, + 'name': 'UserDefinedTypeName' + } + ], + 'name': 'ArrayTypeName' + } + ], + 'name': 'VariableDeclaration' + }, + { + 'attributes': { + 'constant': false, + 'name': 'Ballot', + 'payable': false, + 'visibility': 'public' + }, + 'children': [], + 'name': 'FunctionDefinition' + }, + { + 'attributes': { + 'constant': false, + 'name': 'giveRightToVote', + 'payable': false, + 'visibility': 'public' + }, + 'children': [], + 'name': 'FunctionDefinition' + } + ], + 'name': 'ContractDefinition' + } + var res = common.getStateVariableDeclarationsFormContractNode(contract).map(common.getDeclaredVariableName) + t.comment(res) + t.ok(res[0] === 'chairperson', 'var 1 should be ') + t.ok(res[1] === 'voters', 'var 2 should be ') + t.ok(res[2] === 'proposals', 'var 3 should be ') + t.ok(res[3] === undefined, 'var 4 should be undefined') +}) + +test('staticAnalysisCommon.getFunctionOrModifierDefinitionParameterPart', function (t) { + t.plan(2) + var funDef = { + 'attributes': { + 'constant': true, + 'name': 'winnerName', + 'payable': false, + 'visibility': 'public' + }, + 'children': [ + { + 'children': [ + ], + 'name': 'ParameterList' + }, + { + 'children': [], + 'name': 'ParameterList' + }, + { + 'children': [], + 'name': 'Block' + } + ], + 'name': 'FunctionDefinition' + } + t.ok(common.helpers.nodeType(common.getFunctionOrModifierDefinitionParameterPart(funDef), 'ParameterList'), 'should return a parameterList') + t.throws(() => common.getFunctionOrModifierDefinitionParameterPart({ name: 'SourceUnit' }), undefined, 'throws on other nodes') +}) + +test('staticAnalysisCommon.getFunctionCallTypeParameterType', function (t) { + t.plan(4) + var localCall = { + 'attributes': { + 'type': 'tuple()', + 'type_conversion': false + }, + 'children': [ + { + 'attributes': { + 'type': 'function (struct Ballot.Voter storage pointer)', + 'value': 'bli' + }, + 'id': 37, + 'name': 'Identifier', + 'src': '540:3:0' + }, + { + 'attributes': { + 'type': 'struct Ballot.Voter storage pointer', + 'value': 'x' + }, + 'id': 38, + 'name': 'Identifier', + 'src': '544:1:0' + } + ], + 'id': 39, + 'name': 'FunctionCall', + 'src': '540:6:0' + } + var thisLocalCall = { name: 'MemberAccess', children: [ { attributes: { value: 'this', type: 'contract test' }, name: 'Identifier' } ], attributes: { value: 'b', type: 'function (bytes32,address) returns (bool)' } } + var externalDirect = { + attributes: { + member_name: 'info', + type: 'function () payable external returns (uint256)' + }, + children: [ + { + attributes: { + type: 'contract InfoFeed', + value: 'f' + }, + id: 30, + name: 'Identifier', + src: '405:1:0' + } + ], + id: 32, + name: 'MemberAccess', + src: '405:6:0' + } + t.ok(common.getFunctionCallTypeParameterType(thisLocalCall) === 'bytes32,address', 'this local call returns correct type') + t.ok(common.getFunctionCallTypeParameterType(externalDirect) === '', 'external direct call returns correct type') + t.ok(common.getFunctionCallTypeParameterType(localCall) === 'struct Ballot.Voter storage pointer', 'local call returns correct type') + t.throws(() => common.getFunctionCallTypeParameterType({ name: 'MemberAccess' }), undefined, 'throws on wrong type') +}) + +test('staticAnalysisCommon.getFullQualifiedFunctionCallIdent', function (t) { + t.plan(4) + var contract = { name: 'ContractDefinition', attributes: { name: 'baz' } } + var localCall = { + 'attributes': { + 'type': 'tuple()', + 'type_conversion': false + }, + 'children': [ + { + 'attributes': { + 'type': 'function (struct Ballot.Voter storage pointer)', + 'value': 'bli' + }, + 'id': 37, + 'name': 'Identifier', + 'src': '540:3:0' + }, + { + 'attributes': { + 'type': 'struct Ballot.Voter storage pointer', + 'value': 'x' + }, + 'id': 38, + 'name': 'Identifier', + 'src': '544:1:0' + } + ], + 'id': 39, + 'name': 'FunctionCall', + 'src': '540:6:0' + } + var thisLocalCall = { name: 'MemberAccess', children: [ { attributes: { value: 'this', type: 'contract test' }, name: 'Identifier' } ], attributes: { value: 'b', type: 'function (bytes32,address) returns (bool)' } } + var externalDirect = { + attributes: { + member_name: 'info', + type: 'function () payable external returns (uint256)' + }, + children: [ + { + attributes: { + type: 'contract InfoFeed', + value: 'f' + }, + id: 30, + name: 'Identifier', + src: '405:1:0' + } + ], + id: 32, + name: 'MemberAccess', + src: '405:6:0' + } + + t.ok(common.getFullQualifiedFunctionCallIdent(contract, thisLocalCall) === 'test.b(bytes32,address)', 'this local call returns correct type') + t.ok(common.getFullQualifiedFunctionCallIdent(contract, externalDirect) === 'InfoFeed.info()', 'external direct call returns correct type') + t.ok(common.getFullQualifiedFunctionCallIdent(contract, localCall) === 'baz.bli(struct Ballot.Voter storage pointer)', 'local call returns correct type') + t.throws(() => common.getFullQualifiedFunctionCallIdent(contract, { name: 'MemberAccess' }), undefined, 'throws on wrong type') +}) + +test('staticAnalysisCommon.getFullQuallyfiedFuncDefinitionIdent', function (t) { + t.plan(3) + var contract = { name: 'ContractDefinition', attributes: { name: 'baz' } } + var funDef = { + 'attributes': { + 'constant': false, + 'name': 'getY', + 'payable': false, + 'visibility': 'public' + }, + 'children': [ + { + 'children': [ + { + 'attributes': { + 'name': 'z', + 'type': 'uint256' + }, + 'children': [ + { + 'attributes': { + 'name': 'uint' + }, + 'name': 'ElementaryTypeName' + } + ], + 'name': 'VariableDeclaration' + }, + { + 'attributes': { + 'name': 'r', + 'type': 'bool' + }, + 'children': [ + { + 'attributes': { + 'name': 'bool' + }, + 'name': 'ElementaryTypeName' + } + ], + 'name': 'VariableDeclaration' + } + ], + 'name': 'ParameterList' + }, + { + 'children': [ + { + 'attributes': { + 'name': '', + 'type': 'uint256' + }, + 'children': [ + { + 'attributes': { + 'name': 'uint' + }, + 'id': 34, + 'name': 'ElementaryTypeName', + 'src': '285:4:0' + } + ], + 'id': 35, + 'name': 'VariableDeclaration', + 'src': '285:4:0' + } + ], + 'name': 'ParameterList' + }, + { + 'children': [], + 'name': 'Block' + } + ], + 'name': 'FunctionDefinition' + } + t.ok(common.getFullQuallyfiedFuncDefinitionIdent(contract, funDef, ['uint256', 'bool']) === 'baz.getY(uint256,bool)', 'creates right signature') + t.throws(() => common.getFullQuallyfiedFuncDefinitionIdent(contract, { name: 'MemberAccess' }, ['uint256', 'bool']), undefined, 'throws on wrong nodes') + t.throws(() => common.getFullQuallyfiedFuncDefinitionIdent({ name: 'FunctionCall' }, funDef, ['uint256', 'bool']), undefined, 'throws on wrong nodes') +}) + +// #################### Trivial Node Identification + +test('staticAnalysisCommon.isFunctionDefinition', function (t) { + t.plan(3) + var node1 = { name: 'FunctionDefinition' } + var node2 = { name: 'MemberAccess' } + var node3 = { name: 'FunctionDefinitionBLABLA' } + + t.ok(common.isFunctionDefinition(node1), 'is exact match should work') + t.notOk(common.isFunctionDefinition(node2), 'different node should not work') + t.notOk(common.isFunctionDefinition(node3), 'substring should not work') +}) + +test('staticAnalysisCommon.isModifierDefinition', function (t) { + t.plan(3) + var node1 = { name: 'ModifierDefinition' } + var node2 = { name: 'MemberAccess' } + var node3 = { name: 'ModifierDefinitionBLABLA' } + + t.ok(common.isModifierDefinition(node1), 'is exact match should work') + t.notOk(common.isModifierDefinition(node2), 'different node should not work') + t.notOk(common.isModifierDefinition(node3), 'substring should not work') +}) + +test('staticAnalysisCommon.isModifierInvocation', function (t) { + t.plan(3) + var node1 = { name: 'ModifierInvocation' } + var node2 = { name: 'MemberAccess' } + var node3 = { name: 'ModifierInvocationBLABLA' } + + t.ok(common.isModifierInvocation(node1), 'is exact match should work') + t.notOk(common.isModifierInvocation(node2), 'different node should not work') + t.notOk(common.isModifierInvocation(node3), 'substring should not work') +}) + +test('staticAnalysisCommon.isVariableDeclaration', function (t) { + t.plan(3) + var node1 = { name: 'VariableDeclaration' } + var node2 = { name: 'MemberAccess' } + var node3 = { name: 'VariableDeclarationBLABLA' } + + t.ok(common.isVariableDeclaration(node1), 'is exact match should work') + t.notOk(common.isVariableDeclaration(node2), 'different node should not work') + t.notOk(common.isVariableDeclaration(node3), 'substring should not work') +}) + +test('staticAnalysisCommon.isInheritanceSpecifier', function (t) { + t.plan(3) + var node1 = { name: 'InheritanceSpecifier' } + var node2 = { name: 'MemberAccess' } + var node3 = { name: 'InheritanceSpecifierBLABLA' } + + t.ok(common.isInheritanceSpecifier(node1), 'is exact match should work') + t.notOk(common.isInheritanceSpecifier(node2), 'different node should not work') + t.notOk(common.isInheritanceSpecifier(node3), 'substring should not work') +}) + +test('staticAnalysisCommon.isAssignment', function (t) { + t.plan(3) + var node1 = { name: 'Assignment' } + var node2 = { name: 'MemberAccess' } + var node3 = { name: 'AssignmentBLABLA' } + + t.ok(common.isAssignment(node1), 'is exact match should work') + t.notOk(common.isAssignment(node2), 'different node should not work') + t.notOk(common.isAssignment(node3), 'substring should not work') +}) + +test('staticAnalysisCommon.isContractDefinition', function (t) { + t.plan(3) + var node1 = { name: 'ContractDefinition' } + var node2 = { name: 'MemberAccess' } + var node3 = { name: 'ContractDefinitionBLABLA' } + + t.ok(common.isContractDefinition(node1), 'is exact match should work') + t.notOk(common.isContractDefinition(node2), 'different node should not work') + t.notOk(common.isContractDefinition(node3), 'substring should not work') +}) + +test('staticAnalysisCommon.isInlineAssembly', function (t) { + t.plan(3) + var node1 = { name: 'InlineAssembly' } + var node2 = { name: 'MemberAccess' } + var node3 = { name: 'InlineAssemblyBLABLA' } + + t.ok(common.isInlineAssembly(node1), 'is exact match should work') + t.notOk(common.isInlineAssembly(node2), 'different node should not work') + t.notOk(common.isInlineAssembly(node3), 'substring should not work') +}) + +// #################### Complex Node Identification + +test('staticAnalysisCommon.isStorageVariableDeclaration', function (t) { + t.plan(3) + var node1 = { + 'attributes': { + 'name': 'x', + 'type': 'struct Ballot.Voter storage pointer' + }, + 'children': [ + { + 'attributes': { + 'name': 'Voter' + }, + 'id': 43, + 'name': 'UserDefinedTypeName', + 'src': '604:5:0' + } + ], + 'id': 44, + 'name': 'VariableDeclaration', + 'src': '604:15:0' + } + var node2 = { + 'attributes': { + 'name': 'voters', + 'type': 'mapping(address => struct Ballot.Voter storage ref)' + }, + 'children': [ + { + 'children': [ + { + 'attributes': { + 'name': 'address' + }, + 'id': 16, + 'name': 'ElementaryTypeName', + 'src': '235:7:0' + }, + { + 'attributes': { + 'name': 'Voter' + }, + 'id': 17, + 'name': 'UserDefinedTypeName', + 'src': '246:5:0' + } + ], + 'id': 18, + 'name': 'Mapping', + 'src': '227:25:0' + } + ], + 'id': 19, + 'name': 'VariableDeclaration', + 'src': '227:32:0' + } + var node3 = { + 'attributes': { + 'name': 'voters', + 'type': 'bytes32' + }, + 'children': [ + { + 'attributes': { + 'name': 'bytes' + }, + 'id': 16, + 'name': 'ElementaryTypeName', + 'src': '235:7:0' + } + ], + 'id': 19, + 'name': 'VariableDeclaration', + 'src': '227:32:0' + } + + t.ok(common.isStorageVariableDeclaration(node1), 'struct storage pointer param is storage') + t.ok(common.isStorageVariableDeclaration(node2), 'struct storage pointer mapping param is storage') + t.notOk(common.isStorageVariableDeclaration(node3), 'bytes is not storage') +}) + +test('staticAnalysisCommon.isInteraction', function (t) { + t.plan(6) + 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)' } } + var nodeExtDir = { + attributes: { + member_name: 'info', + type: 'function () payable external returns (uint256)' + }, + children: [ + { + attributes: { + type: 'contract InfoFeed', + value: 'f' + }, + id: 30, + name: 'Identifier', + src: '405:1:0' + } + ], + id: 32, + name: 'MemberAccess', + src: '405:6:0' + } + var nodeNot = { + 'attributes': { + 'type': 'tuple()', + 'type_conversion': false + }, + 'children': [ + { + 'attributes': { + 'type': 'function (struct Ballot.Voter storage pointer)', + 'value': 'bli' + }, + 'id': 37, + 'name': 'Identifier', + 'src': '540:3:0' + }, + { + 'attributes': { + 'type': 'struct Ballot.Voter storage pointer', + 'value': 'x' + }, + 'id': 38, + 'name': 'Identifier', + 'src': '544:1:0' + } + ], + 'id': 39, + 'name': 'FunctionCall', + 'src': '540:6:0' + } + + t.ok(common.isInteraction(sendAst), 'send is interaction') + t.ok(common.isInteraction(callAst), 'call is interaction') + t.ok(common.isInteraction(nodeExtDir), 'ExternalDirecCall is interaction') + t.notOk(common.isInteraction(callcodeAst), 'callcode is not interaction') + t.notOk(common.isInteraction(delegatecallAst), 'callcode is not interaction') + t.notOk(common.isInteraction(nodeNot), 'local call is not interaction') +}) + +test('staticAnalysisCommon.isEffect', function (t) { + t.plan(5) + var inlineAssembly = { + 'children': [ + ], + 'id': 21, + 'name': 'InlineAssembly', + 'src': '809:41:0' + } + var assignment = { + 'attributes': { + 'operator': '=', + 'type': 'uint256' + }, + 'children': [ + { + 'attributes': { + 'type': 'uint256' + }, + 'children': [ + { + 'attributes': { + 'type': 'mapping(address => uint256)', + 'value': 'c' + }, + 'id': 61, + 'name': 'Identifier', + 'src': '873:1:0' + }, + { + 'attributes': { + 'member_name': 'sender', + 'type': 'address' + }, + 'children': [ + { + 'attributes': { + 'type': 'msg', + 'value': 'msg' + }, + 'id': 62, + 'name': 'Identifier', + 'src': '875:3:0' + } + ], + 'id': 63, + 'name': 'MemberAccess', + 'src': '875:10:0' + } + ], + 'id': 64, + 'name': 'IndexAccess', + 'src': '873:13:0' + }, + { + 'attributes': { + 'hexvalue': '30', + 'subdenomination': null, + 'token': null, + 'type': 'int_const 0', + 'value': '0' + }, + 'id': 65, + 'name': 'Literal', + 'src': '889:1:0' + } + ], + 'id': 66, + 'name': 'Assignment', + 'src': '873:17:0' + } + var unaryOp = { name: 'UnaryOperation', attributes: { operator: '++' } } + t.ok(common.isEffect(inlineAssembly), 'inline assembly is treated as effect') + t.ok(common.isEffect(assignment), 'assignment is treated as effect') + t.ok(common.isEffect(unaryOp), '++ is treated as effect') + unaryOp.attributes.operator = '--' + t.ok(common.isEffect(unaryOp), '-- is treated as effect') + t.notOk(common.isEffect({ name: 'MemberAccess', attributes: { operator: '++' } }), 'MemberAccess not treated as effect') +}) + +test('staticAnalysisCommon.isWriteOnStateVariable', function (t) { + t.plan(3) + var inlineAssembly = { + 'children': [ + ], + 'id': 21, + 'name': 'InlineAssembly', + 'src': '809:41:0' + } + var assignment = { + 'attributes': { + 'operator': '=', + 'type': 'uint256' + }, + 'children': [ + { + 'attributes': { + 'type': 'uint256' + }, + 'children': [ + { + 'attributes': { + 'type': 'mapping(address => uint256)', + 'value': 'c' + }, + 'id': 61, + 'name': 'Identifier', + 'src': '873:1:0' + }, + { + 'attributes': { + 'member_name': 'sender', + 'type': 'address' + }, + 'children': [ + { + 'attributes': { + 'type': 'msg', + 'value': 'msg' + }, + 'id': 62, + 'name': 'Identifier', + 'src': '875:3:0' + } + ], + 'id': 63, + 'name': 'MemberAccess', + 'src': '875:10:0' + } + ], + 'id': 64, + 'name': 'IndexAccess', + 'src': '873:13:0' + }, + { + 'attributes': { + 'hexvalue': '30', + 'subdenomination': null, + 'token': null, + 'type': 'int_const 0', + 'value': '0' + }, + 'id': 65, + 'name': 'Literal', + 'src': '889:1:0' + } + ], + 'id': 66, + 'name': 'Assignment', + 'src': '873:17:0' + } + var node1 = { + 'attributes': { + 'name': 'x', + 'type': 'struct Ballot.Voter storage pointer' + }, + 'children': [ + { + 'attributes': { + 'name': 'Voter' + }, + 'name': 'UserDefinedTypeName' + } + ], + 'name': 'VariableDeclaration' + } + var node2 = { + 'attributes': { + 'name': 'y', + 'type': 'uint' + }, + 'children': [ + { + 'attributes': { + 'name': 'Voter' + }, + 'name': 'UserDefinedTypeName' + } + ], + 'name': 'VariableDeclaration' + } + var node3 = { + 'attributes': { + 'name': 'xx', + 'type': 'uint' + }, + 'children': [ + { + 'attributes': { + 'name': 'Voter' + }, + 'name': 'UserDefinedTypeName' + } + ], + 'name': 'VariableDeclaration' + } + t.notOk(common.isWriteOnStateVariable(inlineAssembly, [node1, node2, node3]), 'inline Assembly is not write on state') + t.notOk(common.isWriteOnStateVariable(assignment, [node1, node2, node3]), 'assignment on non state is not write on state') + node3.attributes.name = 'c' + t.ok(common.isWriteOnStateVariable(assignment, [node1, node2, node3]), 'assignment on state is not write on state') +}) + +test('staticAnalysisCommon.isStateVariable', function (t) { + t.plan(3) + var node1 = { + 'attributes': { + 'name': 'x', + 'type': 'struct Ballot.Voter storage pointer' + }, + 'children': [ + { + 'attributes': { + 'name': 'Voter' + }, + 'name': 'UserDefinedTypeName' + } + ], + 'name': 'VariableDeclaration' + } + var node2 = { + 'attributes': { + 'name': 'y', + 'type': 'uint' + }, + 'children': [ + { + 'attributes': { + 'name': 'Voter' + }, + 'name': 'UserDefinedTypeName' + } + ], + 'name': 'VariableDeclaration' + } + var node3 = { + 'attributes': { + 'name': 'xx', + 'type': 'uint' + }, + 'children': [ + { + 'attributes': { + 'name': 'Voter' + }, + 'name': 'UserDefinedTypeName' + } + ], + 'name': 'VariableDeclaration' + } + + t.ok(common.isStateVariable('x', [node1, node2]), 'is contained') + t.ok(common.isStateVariable('x', [node2, node1, node1]), 'is contained twice') + t.notOk(common.isStateVariable('x', [node2, node3]), 'not contained') +}) + +test('staticAnalysisCommon.isConstantFunction', function (t) { + t.plan(3) + var node1 = { name: 'FunctionDefinition', attributes: { constant: true } } + var node2 = { name: 'FunctionDefinition', attributes: { constant: false } } + var node3 = { name: 'MemberAccess', attributes: { constant: true } } + + t.ok(common.isConstantFunction(node1), 'should be const func definition') + t.notOk(common.isConstantFunction(node2), 'should not be const func definition') + t.notOk(common.isConstantFunction(node3), 'wrong node should not be const func definition') +}) + +test('staticAnalysisCommon.isPlusPlusUnaryOperation', function (t) { + t.plan(3) + var node1 = { name: 'UnaryOperation', attributes: { operator: '++' } } + var node2 = { name: 'UnaryOperation', attributes: { operator: '--' } } + var node3 = { name: 'FunctionDefinition', attributes: { operator: '++' } } + + t.ok(common.isPlusPlusUnaryOperation(node1), 'should be unary ++') + t.notOk(common.isPlusPlusUnaryOperation(node2), 'should not be unary ++') + t.notOk(common.isPlusPlusUnaryOperation(node3), 'wrong node should not be unary ++') +}) + +test('staticAnalysisCommon.isMinusMinusUnaryOperation', function (t) { + t.plan(3) + var node1 = { name: 'UnaryOperation', attributes: { operator: '--' } } + var node2 = { name: 'UnaryOperation', attributes: { operator: '++' } } + var node3 = { name: 'FunctionDefinition', attributes: { operator: '--' } } + + t.ok(common.isMinusMinusUnaryOperation(node1), 'should be unary --') + t.notOk(common.isMinusMinusUnaryOperation(node2), 'should not be unary --') + t.notOk(common.isMinusMinusUnaryOperation(node3), 'wrong node should not be unary --') +}) + +test('staticAnalysisCommon.isFullyImplementedContract', function (t) { + t.plan(3) + var node1 = { name: 'ContractDefinition', attributes: { fullyImplemented: true } } + var node2 = { name: 'ContractDefinition', attributes: { fullyImplemented: false } } + var node3 = { name: 'FunctionDefinition', attributes: { operator: '--' } } + + t.ok(common.isFullyImplementedContract(node1), 'should be fully implemented contract') + t.notOk(common.isFullyImplementedContract(node2), 'should not be fully implemented contract') + t.notOk(common.isFullyImplementedContract(node3), 'wrong node should not be fully implemented contract') +}) + +test('staticAnalysisCommon.isCallToNonConstLocalFunction', function (t) { + t.plan(2) + var node1 = { + 'attributes': { + 'type': 'tuple()', + 'type_conversion': false + }, + 'children': [ + { + 'attributes': { + 'type': 'function (struct Ballot.Voter storage pointer)', + 'value': 'bli' + }, + 'id': 37, + 'name': 'Identifier', + 'src': '540:3:0' + }, + { + 'attributes': { + 'type': 'struct Ballot.Voter storage pointer', + 'value': 'x' + }, + 'id': 38, + 'name': 'Identifier', + 'src': '544:1:0' + } + ], + 'id': 39, + 'name': 'FunctionCall', + 'src': '540:6:0' + } + + t.ok(common.isCallToNonConstLocalFunction(node1), 'should be call to non const Local func') + node1.children[0].attributes.type = 'function (struct Ballot.Voter storage pointer) constant payable (uint256)' + t.notok(common.isCallToNonConstLocalFunction(node1), 'should no longer be call to non const Local func') +}) + +test('staticAnalysisCommon.isExternalDirectCall', function (t) { + t.plan(5) + var node = { + attributes: { + member_name: 'info', + type: 'function () payable external returns (uint256)' + }, + children: [ + { + attributes: { + type: 'contract InfoFeed', + value: 'f' + }, + id: 30, + name: 'Identifier', + src: '405:1:0' + } + ], + id: 32, + name: 'MemberAccess', + src: '405:6:0' + } + + var node2 = { name: 'MemberAccess', children: [{attributes: { value: 'this', type: 'contract test' }}], attributes: { value: 'b', type: 'function (bytes32,address) returns (bool)' } } + 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.notOk(common.isNowAccess(node), 'is now used should not work') + t.ok(common.isExternalDirectCall(node), 'f.info() should be external direct call') + t.notOk(common.isExternalDirectCall(node2), 'local call is not an exernal call') +}) + +test('staticAnalysisCommon.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') +}) + +test('staticAnalysisCommon.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.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.isLocalCall', function (t) { + t.plan(5) + var node1 = { + 'attributes': { + 'type': 'tuple()', + 'type_conversion': false + }, + 'children': [ + { + 'attributes': { + 'type': 'function (struct Ballot.Voter storage pointer)', + 'value': 'bli' + }, + 'id': 37, + 'name': 'Identifier', + 'src': '540:3:0' + }, + { + 'attributes': { + 'type': 'struct Ballot.Voter storage pointer', + 'value': 'x' + }, + 'id': 38, + 'name': 'Identifier', + 'src': '544:1:0' + } + ], + 'id': 39, + 'name': 'FunctionCall', + 'src': '540:6:0' + } + + t.ok(common.isLocalCall(node1), 'isLocalCall') + t.notOk(common.isLowLevelCall(node1), 'is not low level call') + t.notOk(common.isExternalDirectCall(node1), 'is not external direct call') + t.notOk(common.isEffect(node1), 'is not effect') + t.notOk(common.isInteraction(node1), 'is not interaction') +}) + +test('staticAnalysisCommon.isLowLevelCall', function (t) { + t.plan(6) + 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.notOk(common.isLowLevelCallInst(callcodeAst), 'callcode is not call') + t.ok(common.isLowLevelCallcodeInst(callcodeAst) && common.isLowLevelCall(callcodeAst), 'callcode is llc should work') + t.notOk(common.isLowLevelCallcodeInst(callAst), 'call is not callcode') + t.ok(common.isLowLevelDelegatecallInst(delegatecallAst) && common.isLowLevelCall(delegatecallAst), 'delegatecall is llc should work') }) From 44327da285c3f4ec77b403905b8e15429efdca26 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michael=20Fr=C3=B6wis?= Date: Mon, 3 Apr 2017 19:05:31 +0200 Subject: [PATCH 03/18] Static Analysis: integration tests --- src/app/staticanalysis/astWalker.js | 54 ++++ .../staticanalysis/modules/abstractAstView.js | 3 +- .../modules/checksEffectsInteraction.js | 2 + .../modules/constantFunctions.js | 2 + src/app/staticanalysis/modules/list.js | 3 + .../modules/staticAnalysisCommon.js | 8 +- .../staticanalysis/staticAnalysisRunner.js | 3 +- .../staticAnalysisCommon-test.js | 9 + .../staticAnalysisIntegration-test.js | 239 ++++++++++++++---- .../test-contracts/assembly.sol | 15 +- 10 files changed, 281 insertions(+), 57 deletions(-) create mode 100644 src/app/staticanalysis/astWalker.js diff --git a/src/app/staticanalysis/astWalker.js b/src/app/staticanalysis/astWalker.js new file mode 100644 index 0000000000..f8737122d6 --- /dev/null +++ b/src/app/staticanalysis/astWalker.js @@ -0,0 +1,54 @@ +'use strict' +/** + * Crawl the given AST through the function walk(ast, callback) + */ +function AstWalker () { +} + +/** + * visit all the AST nodes + * + * @param {Object} ast - AST node + * @param {Object or Function} callback - if (Function) the function will be called for every node. + * - if (Object) callback[] will be called for + * every node of type . callback["*"] will be called fo all other nodes. + * in each case, if the callback returns false it does not descend into children. + * If no callback for the current type, children are visited. + */ +AstWalker.prototype.walk = function (ast, callback) { + if (callback instanceof Function) { + callback = {'*': callback} + } + if (!('*' in callback)) { + callback['*'] = function () { return true } + } + if (manageCallBack(ast, callback) && ast.children && ast.children.length > 0) { + for (var k in ast.children) { + var child = ast.children[k] + this.walk(child, callback) + } + } +} + +/** + * walk the given @astList + * + * @param {Object} sourcesList - sources list (containing root AST node) + * @param {Function} - callback used by AstWalker to compute response + */ +AstWalker.prototype.walkAstList = function (sourcesList, callback) { + var walker = new AstWalker() + for (var k in sourcesList) { + walker.walk(sourcesList[k].AST, callback) + } +} + +function manageCallBack (node, callback) { + if (node.name in callback) { + return callback[node.name](node) + } else { + return callback['*'](node) + } +} + +module.exports = AstWalker diff --git a/src/app/staticanalysis/modules/abstractAstView.js b/src/app/staticanalysis/modules/abstractAstView.js index 7f2a610a45..0d7cc42bca 100644 --- a/src/app/staticanalysis/modules/abstractAstView.js +++ b/src/app/staticanalysis/modules/abstractAstView.js @@ -1,5 +1,6 @@ var common = require('./staticAnalysisCommon') -var AstWalker = require('ethereum-remix').util.AstWalker +// var AstWalker = require('ethereum-remix').util.AstWalker +var AstWalker = require('../astWalker') function abstractAstView () { this.contracts = null diff --git a/src/app/staticanalysis/modules/checksEffectsInteraction.js b/src/app/staticanalysis/modules/checksEffectsInteraction.js index 86f60a2545..7145e48d17 100644 --- a/src/app/staticanalysis/modules/checksEffectsInteraction.js +++ b/src/app/staticanalysis/modules/checksEffectsInteraction.js @@ -19,6 +19,8 @@ checksEffectsInteraction.prototype.report = function (compilationResults) { 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.` }) + this.contracts.forEach((contract) => { contract.functions.forEach((func) => { func.changesState = checkIfChangesState(common.getFullQuallyfiedFuncDefinitionIdent(contract.node, func.node, func.parameters), diff --git a/src/app/staticanalysis/modules/constantFunctions.js b/src/app/staticanalysis/modules/constantFunctions.js index c1c95a9399..8f69e204c8 100644 --- a/src/app/staticanalysis/modules/constantFunctions.js +++ b/src/app/staticanalysis/modules/constantFunctions.js @@ -19,6 +19,8 @@ constantFunctions.prototype.report = function (compilationResults) { 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.` }) + this.contracts.forEach((contract) => { if (!common.isFullyImplementedContract(contract.node)) return diff --git a/src/app/staticanalysis/modules/list.js b/src/app/staticanalysis/modules/list.js index 57ed694688..a27238f8bb 100644 --- a/src/app/staticanalysis/modules/list.js +++ b/src/app/staticanalysis/modules/list.js @@ -5,4 +5,7 @@ module.exports = [ require('./checksEffectsInteraction'), require('./constantFunctions'), require('./inlineAssembly') + // require('./blockTimestamp'), + // require('./lowLevelCalls'), + // require('./blockBlockhash') ] diff --git a/src/app/staticanalysis/modules/staticAnalysisCommon.js b/src/app/staticanalysis/modules/staticAnalysisCommon.js index 8c4190f840..8d01e8719e 100644 --- a/src/app/staticanalysis/modules/staticAnalysisCommon.js +++ b/src/app/staticanalysis/modules/staticAnalysisCommon.js @@ -226,18 +226,21 @@ function isExternalDirectCall (node) { return isMemberAccess(node, basicRegex.EXTERNALFUNCTIONTYPE, undefined, basicRegex.CONTRACTTYPE, undefined) && !isThisLocalCall(node) } -// usage of now special variable function isNowAccess (node) { return nodeType(node, exactMatch(nodeTypes.IDENTIFIER)) && expressionType(node, exactMatch(basicTypes.UINT)) && name(node, exactMatch('now')) } -// usage of block timestamp function isBlockTimestampAccess (node) { return isSpecialVariableAccess(node, specialVariables.BLOCKTIMESTAMP) } +// usage of block timestamp +function isBlockBlockHashAccess (node) { + return isSpecialVariableAccess(node, specialVariables.BLOCKHASH) +} + function isThisLocalCall (node) { return isMemberAccess(node, basicRegex.FUNCTIONTYPE, exactMatch('this'), basicRegex.CONTRACTTYPE, undefined) } @@ -382,6 +385,7 @@ module.exports = { isEffect: isEffect, isNowAccess: isNowAccess, isBlockTimestampAccess: isBlockTimestampAccess, + isBlockBlockHashAccess: isBlockBlockHashAccess, isThisLocalCall: isThisLocalCall, isLocalCall: isLocalCall, isWriteOnStateVariable: isWriteOnStateVariable, diff --git a/src/app/staticanalysis/staticAnalysisRunner.js b/src/app/staticanalysis/staticAnalysisRunner.js index 0583fb30d4..c3855aea0e 100644 --- a/src/app/staticanalysis/staticAnalysisRunner.js +++ b/src/app/staticanalysis/staticAnalysisRunner.js @@ -1,5 +1,6 @@ 'use strict' -var AstWalker = require('ethereum-remix').util.AstWalker +// var AstWalker = require('ethereum-remix').util.AstWalker +var AstWalker = require('./astWalker') var list = require('./modules/list') function staticAnalysisRunner () { diff --git a/test/staticanalysis/staticAnalysisCommon-test.js b/test/staticanalysis/staticAnalysisCommon-test.js index 9d1e85ea2d..cb2396f7be 100644 --- a/test/staticanalysis/staticAnalysisCommon-test.js +++ b/test/staticanalysis/staticAnalysisCommon-test.js @@ -1593,6 +1593,15 @@ test('staticAnalysisCommon.isBlockTimestampAccess', function (t) { t.notOk(common.isNowAccess(node), 'is now used should not work') }) +test('staticAnalysisCommon.isBlockBlockhashAccess', function (t) { + t.plan(4) + var node = { name: 'MemberAccess', children: [{attributes: { value: 'block', type: 'block' }}], attributes: { value: 'blockhash', type: 'bytes32' } } + 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.notOk(common.isNowAccess(node), 'is now used should not work') +}) + test('staticAnalysisCommon.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)' } } diff --git a/test/staticanalysis/staticAnalysisIntegration-test.js b/test/staticanalysis/staticAnalysisIntegration-test.js index d784bddc67..036d212f6d 100644 --- a/test/staticanalysis/staticAnalysisIntegration-test.js +++ b/test/staticanalysis/staticAnalysisIntegration-test.js @@ -1,47 +1,192 @@ -// var test = require('tape') - -// var common = require('../../babelify-src/app/staticanalysis/modules/staticAnalysisCommon') -// var StatRunner = require('../../babelify-src/app/staticanalysis/staticAnalysisRunner') -// var utils = require('../../babelify-src/app/utils') -// var Compiler = require('../../babelify-src/app/compiler') - -// var fs = require('fs') -// var path = require('path') - -// var testFiles = [ -// '/test-contracts/KingOfTheEtherThrone.sol', -// '/test-contracts/assembly.sol', -// '/test-contracts/ballot.sol', -// '/test-contracts/ballot_reentrant.sol', -// '/test-contracts/ballot_withoutWarning.sol', -// '/test-contracts/cross_contract.sol', -// '/test-contracts/inheritance.sol', -// '/test-contracts/notReentrant.sol', -// '/test-contracts/structReentrant.sol', -// '/test-contracts/thisLocal.sol', -// '/test-contracts/modifier1.sol', -// '/test-contracts/modifier2.sol' -// ] - -// test('thisLocal.js', function (t) { -// t.plan(0) - -// var module = require('../../babelify-src/app/staticanalysis/modules/thisLocal') - -// runModuleOnFiles(module, t) -// }) - -// function runModuleOnFiles (module, t) { -// var fakeImport = function (url, cb) { cb('Not implemented') } -// var compiler = new Compiler(fakeImport) -// var statRunner = new StatRunner() - -// testFiles.map((fileName) => { -// var contents = fs.readFileSync(path.join(__dirname, fileName), 'utf8') -// var compres = compiler.compile({ 'test': contents }, 'test') - -// statRunner.runWithModuleList(compres, [{ name: module.name, mod: new module.Module() }], (reports) => { -// reports.map((r) => t.comment(r.warning)) -// }) -// }) -// } +var test = require('tape') + +var StatRunner = require('../../babelify-src/app/staticanalysis/staticAnalysisRunner') +// const util = require('util') + +var solc = require('solc') + +var fs = require('fs') +var path = require('path') + +var testFiles = [ + 'KingOfTheEtherThrone.sol', + 'assembly.sol', + 'ballot.sol', + 'ballot_reentrant.sol', + 'ballot_withoutWarnings.sol', + 'cross_contract.sol', + 'inheritance.sol', + 'modifier1.sol', + 'modifier2.sol', + 'notReentrant.sol', + 'structReentrant.sol', + 'thisLocal.sol' +] + +var testFileAsts = {} + +testFiles.forEach((fileName) => { + var contents = fs.readFileSync(path.join(__dirname, 'test-contracts', fileName), 'utf8') + testFileAsts[fileName] = solc.compile(contents, 0) +}) + +test('Integration test thisLocal.js', function (t) { + t.plan(testFiles.length) + + var module = require('../../babelify-src/app/staticanalysis/modules/thisLocal') + + var lengthCheck = { + 'KingOfTheEtherThrone.sol': 0, + 'assembly.sol': 0, + 'ballot.sol': 0, + 'ballot_reentrant.sol': 1, + 'ballot_withoutWarnings.sol': 0, + 'cross_contract.sol': 0, + 'inheritance.sol': 0, + 'modifier1.sol': 0, + 'modifier2.sol': 0, + 'notReentrant.sol': 0, + 'structReentrant.sol': 0, + 'thisLocal.sol': 1 + } + + runModuleOnFiles(module, t, (file, report) => { + t.equal(report.length, lengthCheck[file], `${file} has right amount of this local warnings`) + }) +}) + +test('Integration test checksEffectsInteraction.js', function (t) { + t.plan(testFiles.length) + + var module = require('../../babelify-src/app/staticanalysis/modules/checksEffectsInteraction') + + var lengthCheck = { + 'KingOfTheEtherThrone.sol': 1, + 'assembly.sol': 0, + 'ballot.sol': 0, + 'ballot_reentrant.sol': 1, + 'ballot_withoutWarnings.sol': 0, + 'cross_contract.sol': 0, + 'inheritance.sol': 1, + 'modifier1.sol': 0, + 'modifier2.sol': 0, + 'notReentrant.sol': 0, + 'structReentrant.sol': 1, + 'thisLocal.sol': 0 + } + + runModuleOnFiles(module, t, (file, report) => { + t.equal(report.length, lengthCheck[file], `${file} has right amount of checks-effects-interaction warnings`) + }) +}) + +test('Integration test constatnFunctions.js', function (t) { + t.plan(testFiles.length) + + var module = require('../../babelify-src/app/staticanalysis/modules/constantFunctions') + + var lengthCheck = { + 'KingOfTheEtherThrone.sol': 0, + 'assembly.sol': 0, + 'ballot.sol': 0, + 'ballot_reentrant.sol': 0, + 'ballot_withoutWarnings.sol': 0, + 'cross_contract.sol': 1, + 'inheritance.sol': 0, + 'modifier1.sol': 1, + 'modifier2.sol': 0, + 'notReentrant.sol': 0, + 'structReentrant.sol': 0, + 'thisLocal.sol': 1 + } + + runModuleOnFiles(module, t, (file, report) => { + t.equal(report.length, lengthCheck[file], `${file} has right amount of constant warnings`) + }) +}) + +test('Integration test constantFunctions.js', function (t) { + t.plan(testFiles.length) + + var module = require('../../babelify-src/app/staticanalysis/modules/inlineAssembly') + + var lengthCheck = { + 'KingOfTheEtherThrone.sol': 0, + 'assembly.sol': 2, + 'ballot.sol': 0, + 'ballot_reentrant.sol': 0, + 'ballot_withoutWarnings.sol': 0, + 'cross_contract.sol': 0, + 'inheritance.sol': 0, + 'modifier1.sol': 0, + 'modifier2.sol': 0, + 'notReentrant.sol': 0, + 'structReentrant.sol': 0, + 'thisLocal.sol': 0 + } + + runModuleOnFiles(module, t, (file, report) => { + t.equal(report.length, lengthCheck[file], `${file} has right amount of inline assembly warnings`) + }) +}) + +test('Integration test txOrigin.js', function (t) { + t.plan(testFiles.length) + + var module = require('../../babelify-src/app/staticanalysis/modules/txOrigin') + + var lengthCheck = { + 'KingOfTheEtherThrone.sol': 0, + 'assembly.sol': 1, + 'ballot.sol': 0, + 'ballot_reentrant.sol': 0, + 'ballot_withoutWarnings.sol': 0, + 'cross_contract.sol': 0, + 'inheritance.sol': 0, + 'modifier1.sol': 0, + 'modifier2.sol': 0, + 'notReentrant.sol': 0, + 'structReentrant.sol': 0, + 'thisLocal.sol': 0 + } + + runModuleOnFiles(module, t, (file, report) => { + t.equal(report.length, lengthCheck[file], `${file} has right amount of tx.origin warnings`) + }) +}) + +test('Integration test gasCosts.js', function (t) { + t.plan(testFiles.length) + + var module = require('../../babelify-src/app/staticanalysis/modules/gasCosts') + + var lengthCheck = { + 'KingOfTheEtherThrone.sol': 2, + 'assembly.sol': 2, + 'ballot.sol': 3, + 'ballot_reentrant.sol': 2, + 'ballot_withoutWarnings.sol': 0, + 'cross_contract.sol': 1, + 'inheritance.sol': 1, + 'modifier1.sol': 0, + 'modifier2.sol': 1, + 'notReentrant.sol': 1, + 'structReentrant.sol': 1, + 'thisLocal.sol': 2 + } + + runModuleOnFiles(module, t, (file, report) => { + t.equal(report.length, lengthCheck[file], `${file} has right amount of gasCost warnings`) + }) +}) + +// #################### Helpers +function runModuleOnFiles (module, t, cb) { + var statRunner = new StatRunner() + + testFiles.forEach((fileName) => { + statRunner.runWithModuleList(testFileAsts[fileName], [{ name: module.name, mod: new module.Module() }], (reports) => { + cb(fileName, reports[0].report) + }) + }) +} diff --git a/test/staticanalysis/test-contracts/assembly.sol b/test/staticanalysis/test-contracts/assembly.sol index d1f63e75e1..a71c7e4a26 100644 --- a/test/staticanalysis/test-contracts/assembly.sol +++ b/test/staticanalysis/test-contracts/assembly.sol @@ -1,7 +1,9 @@ -pragma solidity ^0.4.9; + pragma solidity ^0.4.9; contract test { - function at(address _addr) returns (bytes o_code) { + address owner; + + function at(address _addr) returns (bytes o_code) { assembly { // retrieve the size of the code, this needs assembly let size := extcodesize(_addr) @@ -15,12 +17,13 @@ pragma solidity ^0.4.9; // actually retrieve the code, this needs assembly extcodecopy(_addr, add(o_code, 0x20), 0, size) } - } + } - function bla() { - msg.sender.send(19); + function bla() { + if(tx.origin == owner) + msg.sender.send(19); assembly { } - } + } } 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 04/18] 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') From 130f42faa86b7851641dddd674180609250bedd5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michael=20Fr=C3=B6wis?= Date: Fri, 31 Mar 2017 10:07:11 +0200 Subject: [PATCH 05/18] Static Analysis: more unit tests --- .../modules/staticAnalysisCommon.js | 1 - .../staticAnalysisCommon-test.js | 25 +++++++++++++++++++ 2 files changed, 25 insertions(+), 1 deletion(-) diff --git a/src/app/staticanalysis/modules/staticAnalysisCommon.js b/src/app/staticanalysis/modules/staticAnalysisCommon.js index db5df7bae5..438f209fb3 100644 --- a/src/app/staticanalysis/modules/staticAnalysisCommon.js +++ b/src/app/staticanalysis/modules/staticAnalysisCommon.js @@ -251,7 +251,6 @@ function isBlockTimestampAccess (node) { return isSpecialVariableAccess(node, specialVariables.BLOCKTIMESTAMP) } -// usage of block timestamp function isBlockBlockHashAccess (node) { return isSpecialVariableAccess(node, specialVariables.BLOCKHASH) } diff --git a/test/staticanalysis/staticAnalysisCommon-test.js b/test/staticanalysis/staticAnalysisCommon-test.js index 966fc0e25b..25a30a5f75 100644 --- a/test/staticanalysis/staticAnalysisCommon-test.js +++ b/test/staticanalysis/staticAnalysisCommon-test.js @@ -5,6 +5,8 @@ var utils = require('../../src/app/utils') // #################### helpers Test +// #################### helpers Test + test('staticAnalysisCommon.helpers.buildFunctionSignature', function (t) { t.plan(7) @@ -1066,6 +1068,7 @@ test('staticAnalysisCommon.isInlineAssembly', function (t) { // #################### Complex Node Identification +<<<<<<< c916b65ad0663d676ef59882cc7f19bff322aea9 test('staticAnalysisCommon.isBuiltinFunctionCall', function (t) { t.plan(2) var selfdestruct = { @@ -1119,6 +1122,8 @@ test('staticAnalysisCommon.isBuiltinFunctionCall', function (t) { t.notOk(common.isBuiltinFunctionCall(localCall), 'local call is not builtin') }) +======= +>>>>>>> Static Analysis: more unit tests test('staticAnalysisCommon.isStorageVariableDeclaration', function (t) { t.plan(3) var node1 = { @@ -1575,17 +1580,34 @@ test('staticAnalysisCommon.isCallToNonConstLocalFunction', function (t) { 'type': 'function (struct Ballot.Voter storage pointer)', 'value': 'bli' }, +<<<<<<< c916b65ad0663d676ef59882cc7f19bff322aea9 'name': 'Identifier' +======= + 'id': 37, + 'name': 'Identifier', + 'src': '540:3:0' +>>>>>>> Static Analysis: more unit tests }, { 'attributes': { 'type': 'struct Ballot.Voter storage pointer', 'value': 'x' }, +<<<<<<< c916b65ad0663d676ef59882cc7f19bff322aea9 'name': 'Identifier' } ], 'name': 'FunctionCall' +======= + 'id': 38, + 'name': 'Identifier', + 'src': '544:1:0' + } + ], + 'id': 39, + 'name': 'FunctionCall', + 'src': '540:6:0' +>>>>>>> Static Analysis: more unit tests } t.ok(common.isCallToNonConstLocalFunction(node1), 'should be call to non const Local func') @@ -1640,6 +1662,7 @@ test('staticAnalysisCommon.isBlockTimestampAccess', function (t) { t.notOk(common.isNowAccess(node), 'is now used should not work') }) +<<<<<<< c916b65ad0663d676ef59882cc7f19bff322aea9 test('staticAnalysisCommon.isBlockBlockhashAccess', function (t) { t.plan(4) var node = { @@ -1665,6 +1688,8 @@ test('staticAnalysisCommon.isBlockBlockhashAccess', function (t) { t.notOk(common.isNowAccess(node), 'is now used should not work') }) +======= +>>>>>>> Static Analysis: more unit tests test('staticAnalysisCommon.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)' } } From 242a8f4fecbeff124918b6407d9655d0c499659f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michael=20Fr=C3=B6wis?= Date: Tue, 4 Apr 2017 12:28:45 +0200 Subject: [PATCH 06/18] Static Analysis: fixes after merge --- .../staticAnalysisCommon-test.js | 25 ------------------- 1 file changed, 25 deletions(-) diff --git a/test/staticanalysis/staticAnalysisCommon-test.js b/test/staticanalysis/staticAnalysisCommon-test.js index 25a30a5f75..966fc0e25b 100644 --- a/test/staticanalysis/staticAnalysisCommon-test.js +++ b/test/staticanalysis/staticAnalysisCommon-test.js @@ -5,8 +5,6 @@ var utils = require('../../src/app/utils') // #################### helpers Test -// #################### helpers Test - test('staticAnalysisCommon.helpers.buildFunctionSignature', function (t) { t.plan(7) @@ -1068,7 +1066,6 @@ test('staticAnalysisCommon.isInlineAssembly', function (t) { // #################### Complex Node Identification -<<<<<<< c916b65ad0663d676ef59882cc7f19bff322aea9 test('staticAnalysisCommon.isBuiltinFunctionCall', function (t) { t.plan(2) var selfdestruct = { @@ -1122,8 +1119,6 @@ test('staticAnalysisCommon.isBuiltinFunctionCall', function (t) { t.notOk(common.isBuiltinFunctionCall(localCall), 'local call is not builtin') }) -======= ->>>>>>> Static Analysis: more unit tests test('staticAnalysisCommon.isStorageVariableDeclaration', function (t) { t.plan(3) var node1 = { @@ -1580,34 +1575,17 @@ test('staticAnalysisCommon.isCallToNonConstLocalFunction', function (t) { 'type': 'function (struct Ballot.Voter storage pointer)', 'value': 'bli' }, -<<<<<<< c916b65ad0663d676ef59882cc7f19bff322aea9 'name': 'Identifier' -======= - 'id': 37, - 'name': 'Identifier', - 'src': '540:3:0' ->>>>>>> Static Analysis: more unit tests }, { 'attributes': { 'type': 'struct Ballot.Voter storage pointer', 'value': 'x' }, -<<<<<<< c916b65ad0663d676ef59882cc7f19bff322aea9 'name': 'Identifier' } ], 'name': 'FunctionCall' -======= - 'id': 38, - 'name': 'Identifier', - 'src': '544:1:0' - } - ], - 'id': 39, - 'name': 'FunctionCall', - 'src': '540:6:0' ->>>>>>> Static Analysis: more unit tests } t.ok(common.isCallToNonConstLocalFunction(node1), 'should be call to non const Local func') @@ -1662,7 +1640,6 @@ test('staticAnalysisCommon.isBlockTimestampAccess', function (t) { t.notOk(common.isNowAccess(node), 'is now used should not work') }) -<<<<<<< c916b65ad0663d676ef59882cc7f19bff322aea9 test('staticAnalysisCommon.isBlockBlockhashAccess', function (t) { t.plan(4) var node = { @@ -1688,8 +1665,6 @@ test('staticAnalysisCommon.isBlockBlockhashAccess', function (t) { t.notOk(common.isNowAccess(node), 'is now used should not work') }) -======= ->>>>>>> Static Analysis: more unit tests test('staticAnalysisCommon.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)' } } From b2a5eafa2118af64115a0f59d2bc7d3e4a801c52 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michael=20Fr=C3=B6wis?= Date: Tue, 4 Apr 2017 13:44:17 +0200 Subject: [PATCH 07/18] Static Analysis: fix integration tests --- .../staticAnalysisIntegration-test.js | 20 +++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/test/staticanalysis/staticAnalysisIntegration-test.js b/test/staticanalysis/staticAnalysisIntegration-test.js index da0adba732..8a7563ea82 100644 --- a/test/staticanalysis/staticAnalysisIntegration-test.js +++ b/test/staticanalysis/staticAnalysisIntegration-test.js @@ -33,7 +33,7 @@ testFiles.forEach((fileName) => { test('Integration test thisLocal.js', function (t) { t.plan(testFiles.length) - var module = require('../../babelify-src/app/staticanalysis/modules/thisLocal') + var module = require('../../src/app/staticanalysis/modules/thisLocal') var lengthCheck = { 'KingOfTheEtherThrone.sol': 0, @@ -58,7 +58,7 @@ test('Integration test thisLocal.js', function (t) { test('Integration test checksEffectsInteraction.js', function (t) { t.plan(testFiles.length) - var module = require('../../babelify-src/app/staticanalysis/modules/checksEffectsInteraction') + var module = require('../../src/app/staticanalysis/modules/checksEffectsInteraction') var lengthCheck = { 'KingOfTheEtherThrone.sol': 1, @@ -68,8 +68,8 @@ test('Integration test checksEffectsInteraction.js', function (t) { 'ballot_withoutWarnings.sol': 0, 'cross_contract.sol': 0, 'inheritance.sol': 1, - 'modifier1.sol': 0, - 'modifier2.sol': 0, + 'modifier1.sol': 1, + 'modifier2.sol': 1, 'notReentrant.sol': 0, 'structReentrant.sol': 1, 'thisLocal.sol': 0 @@ -83,7 +83,7 @@ test('Integration test checksEffectsInteraction.js', function (t) { test('Integration test constatnFunctions.js', function (t) { t.plan(testFiles.length) - var module = require('../../babelify-src/app/staticanalysis/modules/constantFunctions') + var module = require('../../src/app/staticanalysis/modules/constantFunctions') var lengthCheck = { 'KingOfTheEtherThrone.sol': 0, @@ -93,8 +93,8 @@ test('Integration test constatnFunctions.js', function (t) { 'ballot_withoutWarnings.sol': 0, 'cross_contract.sol': 1, 'inheritance.sol': 0, - 'modifier1.sol': 1, - 'modifier2.sol': 0, + 'modifier1.sol': 2, + 'modifier2.sol': 1, 'notReentrant.sol': 0, 'structReentrant.sol': 0, 'thisLocal.sol': 1 @@ -108,7 +108,7 @@ test('Integration test constatnFunctions.js', function (t) { test('Integration test constantFunctions.js', function (t) { t.plan(testFiles.length) - var module = require('../../babelify-src/app/staticanalysis/modules/inlineAssembly') + var module = require('../../src/app/staticanalysis/modules/inlineAssembly') var lengthCheck = { 'KingOfTheEtherThrone.sol': 0, @@ -133,7 +133,7 @@ test('Integration test constantFunctions.js', function (t) { test('Integration test txOrigin.js', function (t) { t.plan(testFiles.length) - var module = require('../../babelify-src/app/staticanalysis/modules/txOrigin') + var module = require('../../src/app/staticanalysis/modules/txOrigin') var lengthCheck = { 'KingOfTheEtherThrone.sol': 0, @@ -158,7 +158,7 @@ test('Integration test txOrigin.js', function (t) { test('Integration test gasCosts.js', function (t) { t.plan(testFiles.length) - var module = require('../../babelify-src/app/staticanalysis/modules/gasCosts') + var module = require('../../src/app/staticanalysis/modules/gasCosts') var lengthCheck = { 'KingOfTheEtherThrone.sol': 2, From f9f03813d0f1aa0ecf76190646f39e39dd4c3fb9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michael=20Fr=C3=B6wis?= Date: Wed, 5 Apr 2017 17:11:44 +0200 Subject: [PATCH 08/18] Static Analysis: Builtin Functions, Super keyword --- .../modules/checksEffectsInteraction.js | 4 +- .../modules/constantFunctions.js | 2 +- .../modules/functionCallGraph.js | 22 ++--- src/app/staticanalysis/modules/gasCosts.js | 4 +- .../modules/staticAnalysisCommon.js | 23 ++++- src/app/staticanalysis/modules/thisLocal.js | 4 +- src/app/staticanalysis/modules/txOrigin.js | 6 +- .../staticAnalysisCommon-test.js | 99 ++++++++++++++++++- .../staticAnalysisIntegration-test.js | 25 +++-- .../staticanalysis/test-contracts/globals.sol | 57 +++++++++++ 10 files changed, 211 insertions(+), 35 deletions(-) create mode 100644 test/staticanalysis/test-contracts/globals.sol diff --git a/src/app/staticanalysis/modules/checksEffectsInteraction.js b/src/app/staticanalysis/modules/checksEffectsInteraction.js index 07ed9661a5..8958c1238f 100644 --- a/src/app/staticanalysis/modules/checksEffectsInteraction.js +++ b/src/app/staticanalysis/modules/checksEffectsInteraction.js @@ -9,7 +9,7 @@ function checksEffectsInteraction () { this.contracts = [] checksEffectsInteraction.prototype.visit = new AbstractAst().builder( - (node) => common.isInteraction(node) || common.isEffect(node) || (common.isLocalCall(node) && !common.isBuiltinFunctionCall(node)), + (node) => common.isInteraction(node) || common.isEffect(node) || common.isLocalCallGraphRelevantNode(node), this.contracts ) } @@ -64,7 +64,7 @@ function isPotentialVulnerableFunction (func, context) { } function isLocalCallWithStateChange (node, context) { - if (common.isLocalCall(node)) { + if (common.isLocalCallGraphRelevantNode(node)) { var func = callGraph.resolveCallGraphSymbol(context.cg, common.getFullQualifiedFunctionCallIdent(context.currentContract.node, node)) return !func || (func && func.node.changesState) } diff --git a/src/app/staticanalysis/modules/constantFunctions.js b/src/app/staticanalysis/modules/constantFunctions.js index 2075e47107..2b440ea1b0 100644 --- a/src/app/staticanalysis/modules/constantFunctions.js +++ b/src/app/staticanalysis/modules/constantFunctions.js @@ -9,7 +9,7 @@ function constantFunctions () { 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), + (node) => common.isLowLevelCall(node) || common.isExternalDirectCall(node) || common.isEffect(node) || common.isLocalCallGraphRelevantNode(node) || common.isInlineAssembly(node), this.contracts ) } diff --git a/src/app/staticanalysis/modules/functionCallGraph.js b/src/app/staticanalysis/modules/functionCallGraph.js index 1e52da7593..6ec502e766 100644 --- a/src/app/staticanalysis/modules/functionCallGraph.js +++ b/src/app/staticanalysis/modules/functionCallGraph.js @@ -19,7 +19,7 @@ function buildLocalFuncCallGraphInternal (functions, nodeFilter, extractNodeIden function buildGlobalFuncCallGraph (contracts) { var callGraph = {} contracts.forEach((contract) => { - var filterNodes = (node) => { return common.isLocalCall(node) || common.isThisLocalCall(node) || common.isExternalDirectCall(node) } + var filterNodes = (node) => { return common.isLocalCallGraphRelevantNode(node) || common.isExternalDirectCall(node) } var getNodeIdent = (node) => { return common.getFullQualifiedFunctionCallIdent(contract.node, node) } var getFunDefIdent = (funcDef) => { return common.getFullQuallyfiedFuncDefinitionIdent(contract.node, funcDef.node, funcDef.parameters) } @@ -36,7 +36,7 @@ function analyseCallGraph (cg, funcName, context, nodeCheck) { function analyseCallGraphInternal (cg, funcName, context, combinator, nodeCheck, visited) { var current = resolveCallGraphSymbol(cg, funcName) - if (!current || visited[funcName]) return true + if (current === undefined || visited[funcName] === true) return true visited[funcName] = true return combinator(current.node.relevantNodes.reduce((acc, val) => combinator(acc, nodeCheck(val, context)), false), @@ -48,33 +48,31 @@ function resolveCallGraphSymbol (cg, funcName) { } function resolveCallGraphSymbolInternal (cg, funcName, silent) { - var current = null + var current if (funcName.includes('.')) { var parts = funcName.split('.') var contractPart = parts[0] var functionPart = parts[1] var currentContract = cg[contractPart] - if (currentContract) { + if (!(currentContract === undefined)) { current = currentContract.functions[funcName] // resolve inheritance hierarchy - if (!current) { + if (current === undefined) { // resolve inheritance lookup in linearized fashion var inheritsFromNames = currentContract.contract.inheritsFrom.reverse() for (var i = 0; i < inheritsFromNames.length; i++) { var res = resolveCallGraphSymbolInternal(cg, inheritsFromNames[i] + '.' + functionPart, true) - if (res) return res + if (!(res === undefined)) return res } } + } else { + if (!silent) console.log(`static analysis functionCallGraph.js: Contract ${contractPart} not found in function call graph.`) } } else { throw new Error('functionCallGraph.js: function does not have full qualified name.') } - if (!current) { - if (!silent) console.log(`static analysis functionCallGraph.js: ${funcName} not found in function call graph.`) - return null - } else { - return current - } + if (current === undefined && !silent) console.log(`static analysis functionCallGraph.js: ${funcName} not found in function call graph.`) + return current } module.exports = { diff --git a/src/app/staticanalysis/modules/gasCosts.js b/src/app/staticanalysis/modules/gasCosts.js index 3f542bfc80..d850f6eddd 100644 --- a/src/app/staticanalysis/modules/gasCosts.js +++ b/src/app/staticanalysis/modules/gasCosts.js @@ -1,5 +1,5 @@ -var name = 'gas costs: ' -var desc = 'warn if the gas requiremets of functions are too high' +var name = 'Gas Costs: ' +var desc = 'Warn if the gas requiremets of functions are too high.' var categories = require('./categories') function gasCosts () { diff --git a/src/app/staticanalysis/modules/staticAnalysisCommon.js b/src/app/staticanalysis/modules/staticAnalysisCommon.js index 438f209fb3..9643bb553c 100644 --- a/src/app/staticanalysis/modules/staticAnalysisCommon.js +++ b/src/app/staticanalysis/modules/staticAnalysisCommon.js @@ -77,8 +77,8 @@ function getType (node) { // #################### Complex Getters function getFunctionCallType (func) { - if (!(isExternalDirectCall(func) || isThisLocalCall(func) || isLocalCall(func))) throw new Error('staticAnalysisCommon.js: not function call Node') - if (isExternalDirectCall(func) || isThisLocalCall(func)) return func.attributes.type + if (!(isExternalDirectCall(func) || isThisLocalCall(func) || isSuperLocalCall(func) || isLocalCall(func))) throw new Error('staticAnalysisCommon.js: not function call Node') + if (isExternalDirectCall(func) || isThisLocalCall(func) || isSuperLocalCall(func)) return func.attributes.type return findFirstSubNodeLTR(func, exactMatch(nodeTypes.IDENTIFIER)).attributes.type } @@ -97,6 +97,11 @@ function getThisLocalCallName (localCallNode) { return localCallNode.attributes.value } +function getSuperLocalCallName (localCallNode) { + if (!isSuperLocalCall(localCallNode)) throw new Error('staticAnalysisCommon.js: not an super local call Node') + return localCallNode.attributes.member_name +} + function getExternalDirectCallContractName (extDirectCall) { if (!isExternalDirectCall(extDirectCall)) throw new Error('staticAnalysisCommon.js: not an external direct call Node') return extDirectCall.children[0].attributes.type.replace(new RegExp(basicRegex.CONTRACTTYPE), '') @@ -149,6 +154,7 @@ function getFunctionCallTypeParameterType (func) { function getFullQualifiedFunctionCallIdent (contract, func) { if (isLocalCall(func)) return getContractName(contract) + '.' + getLocalCallName(func) + '(' + getFunctionCallTypeParameterType(func) + ')' else if (isThisLocalCall(func)) return getThisLocalCallContractName(func) + '.' + getThisLocalCallName(func) + '(' + getFunctionCallTypeParameterType(func) + ')' + else if (isSuperLocalCall(func)) return getContractName(contract) + '.' + getSuperLocalCallName(func) + '(' + getFunctionCallTypeParameterType(func) + ')' else if (isExternalDirectCall(func)) return getExternalDirectCallContractName(func) + '.' + getExternalDirectCallMemberName(func) + '(' + getFunctionCallTypeParameterType(func) + ')' else throw new Error('staticAnalysisCommon.js: Can not get function name form non function call node') } @@ -193,6 +199,10 @@ function isInlineAssembly (node) { // #################### Complex Node Identification +function isLocalCallGraphRelevantNode (node) { + return ((isLocalCall(node) || isSuperLocalCall(node)) && !isBuiltinFunctionCall(node)) +} + function isBuiltinFunctionCall (node) { return isLocalCall(node) && builtinFunctions[getLocalCallName(node) + '(' + getFunctionCallTypeParameterType(node) + ')'] === true } @@ -238,7 +248,7 @@ function isCallToNonConstLocalFunction (node) { } function isExternalDirectCall (node) { - return isMemberAccess(node, basicRegex.EXTERNALFUNCTIONTYPE, undefined, basicRegex.CONTRACTTYPE, undefined) && !isThisLocalCall(node) + return isMemberAccess(node, basicRegex.EXTERNALFUNCTIONTYPE, undefined, basicRegex.CONTRACTTYPE, undefined) && !isThisLocalCall(node) && !isSuperLocalCall(node) } function isNowAccess (node) { @@ -259,6 +269,10 @@ function isThisLocalCall (node) { return isMemberAccess(node, basicRegex.FUNCTIONTYPE, exactMatch('this'), basicRegex.CONTRACTTYPE, undefined) } +function isSuperLocalCall (node) { + return isMemberAccess(node, basicRegex.FUNCTIONTYPE, exactMatch('super'), basicRegex.CONTRACTTYPE, undefined) +} + function isLocalCall (node) { return nodeType(node, exactMatch(nodeTypes.FUNCTIONCALL)) && minNrOfChildren(node, 1) && @@ -377,6 +391,7 @@ module.exports = { getType: getType, // #################### Complex Getters getThisLocalCallName: getThisLocalCallName, + getSuperLocalCallName: getSuperLocalCallName, getFunctionCallType: getFunctionCallType, getContractName: getContractName, getEffectedVariableName: getEffectedVariableName, @@ -400,6 +415,8 @@ module.exports = { isBlockTimestampAccess: isBlockTimestampAccess, isBlockBlockHashAccess: isBlockBlockHashAccess, isThisLocalCall: isThisLocalCall, + isSuperLocalCall: isSuperLocalCall, + isLocalCallGraphRelevantNode: isLocalCallGraphRelevantNode, isLocalCall: isLocalCall, isWriteOnStateVariable: isWriteOnStateVariable, isStateVariable: isStateVariable, diff --git a/src/app/staticanalysis/modules/thisLocal.js b/src/app/staticanalysis/modules/thisLocal.js index 9369a84c0a..88e906df5a 100644 --- a/src/app/staticanalysis/modules/thisLocal.js +++ b/src/app/staticanalysis/modules/thisLocal.js @@ -1,5 +1,5 @@ -var name = 'this on local: ' -var desc = 'invocation of local functions via this' +var name = 'this on local calls: ' +var desc = 'Invocation of local functions via this' var categories = require('./categories') var common = require('./staticAnalysisCommon') diff --git a/src/app/staticanalysis/modules/txOrigin.js b/src/app/staticanalysis/modules/txOrigin.js index 36d8913237..df5213617b 100644 --- a/src/app/staticanalysis/modules/txOrigin.js +++ b/src/app/staticanalysis/modules/txOrigin.js @@ -1,5 +1,5 @@ -var name = 'tx origin: ' -var desc = 'warn if tx.origin is used' +var name = 'tx.origin: ' +var desc = 'Warn if tx.origin is used' var categories = require('./categories') function txOrigin () { @@ -20,7 +20,7 @@ txOrigin.prototype.visit = function (node) { txOrigin.prototype.report = function () { return this.txOriginNodes.map(function (item, i) { return { - warning: `use of tx.origin: "tx.origin" is useful only in very exceptional cases.
+ warning: `Use of tx.origin: "tx.origin" is useful only in very exceptional cases.
If you use it for authentication, you usually want to replace it by "msg.sender", because otherwise any contract you call can act on your behalf.`, location: item.src } diff --git a/test/staticanalysis/staticAnalysisCommon-test.js b/test/staticanalysis/staticAnalysisCommon-test.js index 966fc0e25b..f1faf240b9 100644 --- a/test/staticanalysis/staticAnalysisCommon-test.js +++ b/test/staticanalysis/staticAnalysisCommon-test.js @@ -394,6 +394,80 @@ test('staticAnalysisCommon.getThisLocalCallName', function (t) { t.throws(() => common.getThisLocalCallName(localCall), undefined, 'throws on other nodes') }) +test('staticAnalysisCommon.getSuperLocalCallName', function (t) { + t.plan(4) + var superLocal = { + 'attributes': { + 'member_name': 'duper', + 'type': 'function ()' + }, + 'children': [ + { + 'attributes': { + 'type': 'contract super a', + 'value': 'super' + }, + 'name': 'Identifier' + } + ], + 'name': 'MemberAccess' + } + var localCall = { + 'attributes': { + 'type': 'tuple()', + 'type_conversion': false + }, + 'children': [ + { + 'attributes': { + 'type': 'function (struct Ballot.Voter storage pointer)', + 'value': 'bli' + }, + 'id': 37, + 'name': 'Identifier', + 'src': '540:3:0' + }, + { + 'attributes': { + 'type': 'struct Ballot.Voter storage pointer', + 'value': 'x' + }, + 'id': 38, + 'name': 'Identifier', + 'src': '544:1:0' + } + ], + 'id': 39, + 'name': 'FunctionCall', + 'src': '540:6:0' + } + var thisLocalCall = { name: 'MemberAccess', children: [ { attributes: { value: 'this', type: 'contract test' }, name: 'Identifier' } ], attributes: { value: 'b', type: 'function (bytes32,address) returns (bool)' } } + var externalDirect = { + attributes: { + member_name: 'info', + type: 'function () payable external returns (uint256)' + }, + children: [ + { + attributes: { + type: 'contract InfoFeed', + value: 'f' + }, + id: 30, + name: 'Identifier', + src: '405:1:0' + } + ], + id: 32, + name: 'MemberAccess', + src: '405:6:0' + } + t.equal(common.getSuperLocalCallName(superLocal), 'duper', 'get local name from super local call') + t.throws(() => common.getSuperLocalCallName(thisLocalCall), 'throws on other nodes') + t.throws(() => common.getSuperLocalCallName(externalDirect), undefined, 'throws on other nodes') + t.throws(() => common.getSuperLocalCallName(localCall), undefined, 'throws on other nodes') +}) + test('staticAnalysisCommon.getExternalDirectCallContractName', function (t) { t.plan(3) var localCall = { @@ -737,7 +811,6 @@ test('staticAnalysisCommon.getStateVariableDeclarationsFormContractNode', functi 'name': 'ContractDefinition' } var res = common.getStateVariableDeclarationsFormContractNode(contract).map(common.getDeclaredVariableName) - t.comment(res) t.ok(res[0] === 'chairperson', 'var 1 should be ') t.ok(res[1] === 'voters', 'var 2 should be ') t.ok(res[2] === 'proposals', 'var 3 should be ') @@ -1673,6 +1746,30 @@ test('staticAnalysisCommon.isThisLocalCall', function (t) { t.notOk(common.isNowAccess(node), 'is now used should not work') }) +test('staticAnalysisCommon.isSuperLocalCall', function (t) { + t.plan(4) + var node = { + 'attributes': { + 'member_name': 'duper', + 'type': 'function ()' + }, + 'children': [ + { + 'attributes': { + 'type': 'contract super a', + 'value': 'super' + }, + 'name': 'Identifier' + } + ], + 'name': 'MemberAccess' + } + t.ok(common.isSuperLocalCall(node), 'is super.local_method() used should work') + 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.notOk(common.isNowAccess(node), 'is now used should not work') +}) + test('staticAnalysisCommon.isLocalCall', function (t) { t.plan(5) var node1 = { diff --git a/test/staticanalysis/staticAnalysisIntegration-test.js b/test/staticanalysis/staticAnalysisIntegration-test.js index 8a7563ea82..3eba1fb617 100644 --- a/test/staticanalysis/staticAnalysisIntegration-test.js +++ b/test/staticanalysis/staticAnalysisIntegration-test.js @@ -20,7 +20,8 @@ var testFiles = [ 'modifier2.sol', 'notReentrant.sol', 'structReentrant.sol', - 'thisLocal.sol' + 'thisLocal.sol', + 'globals.sol' ] var testFileAsts = {} @@ -47,7 +48,8 @@ test('Integration test thisLocal.js', function (t) { 'modifier2.sol': 0, 'notReentrant.sol': 0, 'structReentrant.sol': 0, - 'thisLocal.sol': 1 + 'thisLocal.sol': 1, + 'globals.sol': 0 } runModuleOnFiles(module, t, (file, report) => { @@ -72,7 +74,8 @@ test('Integration test checksEffectsInteraction.js', function (t) { 'modifier2.sol': 1, 'notReentrant.sol': 0, 'structReentrant.sol': 1, - 'thisLocal.sol': 0 + 'thisLocal.sol': 0, + 'globals.sol': 1 } runModuleOnFiles(module, t, (file, report) => { @@ -80,7 +83,7 @@ test('Integration test checksEffectsInteraction.js', function (t) { }) }) -test('Integration test constatnFunctions.js', function (t) { +test('Integration test constantFunctions.js', function (t) { t.plan(testFiles.length) var module = require('../../src/app/staticanalysis/modules/constantFunctions') @@ -97,7 +100,8 @@ test('Integration test constatnFunctions.js', function (t) { 'modifier2.sol': 1, 'notReentrant.sol': 0, 'structReentrant.sol': 0, - 'thisLocal.sol': 1 + 'thisLocal.sol': 1, + 'globals.sol': 0 } runModuleOnFiles(module, t, (file, report) => { @@ -105,7 +109,7 @@ test('Integration test constatnFunctions.js', function (t) { }) }) -test('Integration test constantFunctions.js', function (t) { +test('Integration test inlineAssembly.js', function (t) { t.plan(testFiles.length) var module = require('../../src/app/staticanalysis/modules/inlineAssembly') @@ -122,7 +126,8 @@ test('Integration test constantFunctions.js', function (t) { 'modifier2.sol': 0, 'notReentrant.sol': 0, 'structReentrant.sol': 0, - 'thisLocal.sol': 0 + 'thisLocal.sol': 0, + 'globals.sol': 0 } runModuleOnFiles(module, t, (file, report) => { @@ -147,7 +152,8 @@ test('Integration test txOrigin.js', function (t) { 'modifier2.sol': 0, 'notReentrant.sol': 0, 'structReentrant.sol': 0, - 'thisLocal.sol': 0 + 'thisLocal.sol': 0, + 'globals.sol': 1 } runModuleOnFiles(module, t, (file, report) => { @@ -172,7 +178,8 @@ test('Integration test gasCosts.js', function (t) { 'modifier2.sol': 1, 'notReentrant.sol': 1, 'structReentrant.sol': 1, - 'thisLocal.sol': 2 + 'thisLocal.sol': 2, + 'globals.sol': 1 } runModuleOnFiles(module, t, (file, report) => { diff --git a/test/staticanalysis/test-contracts/globals.sol b/test/staticanalysis/test-contracts/globals.sol new file mode 100644 index 0000000000..c945f84bba --- /dev/null +++ b/test/staticanalysis/test-contracts/globals.sol @@ -0,0 +1,57 @@ +pragma solidity ^0.4.9; +contract bla { + uint brr; + function duper() { + brr++; + } +} + +contract a is bla { + + function blub() { + brr++; + } + + function r () { + address a; + bytes32 hash; + uint8 v; + bytes32 r; + bytes32 s; + + block.blockhash(1); + block.coinbase; + block.difficulty; + block.gaslimit; + block.number; + block.timestamp; + msg.data; + msg.gas; + msg.sender; + msg.value; + now; + tx.gasprice; + tx.origin; + //assert(now == block.timestamp); + //require(now == block.timestamp); + keccak256(a); + sha3(a); + sha256(a); + ripemd160(a); + ecrecover(hash, v, r, s); + addmod(1, 2, 2); + mulmod(4,4,12); + + a.balance; + blub(); + a.send(a.balance); + + + + super.duper(); + //a.transfer(a.balance); + selfdestruct(a); + //revert(); + } + +} \ No newline at end of file From b8e2db29b947b39142b613a2e925175f60014837 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michael=20Fr=C3=B6wis?= Date: Fri, 31 Mar 2017 10:07:11 +0200 Subject: [PATCH 09/18] Static Analysis: more unit tests --- test/staticanalysis/staticAnalysisCommon-test.js | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/test/staticanalysis/staticAnalysisCommon-test.js b/test/staticanalysis/staticAnalysisCommon-test.js index f1faf240b9..fe56a6364f 100644 --- a/test/staticanalysis/staticAnalysisCommon-test.js +++ b/test/staticanalysis/staticAnalysisCommon-test.js @@ -412,6 +412,9 @@ test('staticAnalysisCommon.getSuperLocalCallName', function (t) { ], 'name': 'MemberAccess' } + +test('staticAnalysisCommon.getExternalDirectCallContractName', function (t) { + t.plan(3) var localCall = { 'attributes': { 'type': 'tuple()', @@ -520,6 +523,7 @@ test('staticAnalysisCommon.getExternalDirectCallContractName', function (t) { name: 'MemberAccess', src: '405:6:0' } + t.ok(common.getExternalDirectCallContractName(externalDirect) === 'InfoFeed', 'external direct call contract name from node') t.throws(() => common.getExternalDirectCallContractName(thisLocalCall), undefined, 'throws on other nodes') t.throws(() => common.getExternalDirectCallContractName(localCall), undefined, 'throws on other nodes') @@ -811,6 +815,7 @@ test('staticAnalysisCommon.getStateVariableDeclarationsFormContractNode', functi 'name': 'ContractDefinition' } var res = common.getStateVariableDeclarationsFormContractNode(contract).map(common.getDeclaredVariableName) + t.ok(res[0] === 'chairperson', 'var 1 should be ') t.ok(res[1] === 'voters', 'var 2 should be ') t.ok(res[2] === 'proposals', 'var 3 should be ') @@ -1192,6 +1197,7 @@ test('staticAnalysisCommon.isBuiltinFunctionCall', function (t) { t.notOk(common.isBuiltinFunctionCall(localCall), 'local call is not builtin') }) + test('staticAnalysisCommon.isStorageVariableDeclaration', function (t) { t.plan(3) var node1 = { @@ -1713,6 +1719,7 @@ test('staticAnalysisCommon.isBlockTimestampAccess', function (t) { t.notOk(common.isNowAccess(node), 'is now used should not work') }) +<<<<<<< 1ca54743848c4b5087105325dbc095c1b6a8b428 test('staticAnalysisCommon.isBlockBlockhashAccess', function (t) { t.plan(4) var node = { From 0ef31dfb1bb277a86effed8b7f71b1d5710e0319 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michael=20Fr=C3=B6wis?= Date: Thu, 6 Apr 2017 17:05:42 +0200 Subject: [PATCH 10/18] Static Analysis: use astWalker from ethereum-remix --- src/app/staticanalysis/astWalker.js | 54 ------------------- .../staticanalysis/modules/abstractAstView.js | 3 +- .../staticanalysis/staticAnalysisRunner.js | 3 +- 3 files changed, 2 insertions(+), 58 deletions(-) delete mode 100644 src/app/staticanalysis/astWalker.js diff --git a/src/app/staticanalysis/astWalker.js b/src/app/staticanalysis/astWalker.js deleted file mode 100644 index f8737122d6..0000000000 --- a/src/app/staticanalysis/astWalker.js +++ /dev/null @@ -1,54 +0,0 @@ -'use strict' -/** - * Crawl the given AST through the function walk(ast, callback) - */ -function AstWalker () { -} - -/** - * visit all the AST nodes - * - * @param {Object} ast - AST node - * @param {Object or Function} callback - if (Function) the function will be called for every node. - * - if (Object) callback[] will be called for - * every node of type . callback["*"] will be called fo all other nodes. - * in each case, if the callback returns false it does not descend into children. - * If no callback for the current type, children are visited. - */ -AstWalker.prototype.walk = function (ast, callback) { - if (callback instanceof Function) { - callback = {'*': callback} - } - if (!('*' in callback)) { - callback['*'] = function () { return true } - } - if (manageCallBack(ast, callback) && ast.children && ast.children.length > 0) { - for (var k in ast.children) { - var child = ast.children[k] - this.walk(child, callback) - } - } -} - -/** - * walk the given @astList - * - * @param {Object} sourcesList - sources list (containing root AST node) - * @param {Function} - callback used by AstWalker to compute response - */ -AstWalker.prototype.walkAstList = function (sourcesList, callback) { - var walker = new AstWalker() - for (var k in sourcesList) { - walker.walk(sourcesList[k].AST, callback) - } -} - -function manageCallBack (node, callback) { - if (node.name in callback) { - return callback[node.name](node) - } else { - return callback['*'](node) - } -} - -module.exports = AstWalker diff --git a/src/app/staticanalysis/modules/abstractAstView.js b/src/app/staticanalysis/modules/abstractAstView.js index 0d7cc42bca..7f2a610a45 100644 --- a/src/app/staticanalysis/modules/abstractAstView.js +++ b/src/app/staticanalysis/modules/abstractAstView.js @@ -1,6 +1,5 @@ var common = require('./staticAnalysisCommon') -// var AstWalker = require('ethereum-remix').util.AstWalker -var AstWalker = require('../astWalker') +var AstWalker = require('ethereum-remix').util.AstWalker function abstractAstView () { this.contracts = null diff --git a/src/app/staticanalysis/staticAnalysisRunner.js b/src/app/staticanalysis/staticAnalysisRunner.js index c3855aea0e..0583fb30d4 100644 --- a/src/app/staticanalysis/staticAnalysisRunner.js +++ b/src/app/staticanalysis/staticAnalysisRunner.js @@ -1,6 +1,5 @@ 'use strict' -// var AstWalker = require('ethereum-remix').util.AstWalker -var AstWalker = require('./astWalker') +var AstWalker = require('ethereum-remix').util.AstWalker var list = require('./modules/list') function staticAnalysisRunner () { From f2068ea2362e253d19d9c0943a8f344518aa5e61 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michael=20Fr=C3=B6wis?= Date: Thu, 6 Apr 2017 17:13:49 +0200 Subject: [PATCH 11/18] General: node 7 for testing --- .travis.yml | 2 +- README.md | 2 ++ 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/.travis.yml b/.travis.yml index 7ee87612d5..7dc600a8bd 100644 --- a/.travis.yml +++ b/.travis.yml @@ -1,6 +1,6 @@ language: node_js node_js: - - stable + - "7" script: - npm run lint && npm run test && npm run downloadsolc && npm run make-mock-compiler && npm run build - ./ci/browser_tests.sh diff --git a/README.md b/README.md index ac2dbf2751..74af5e1d9b 100644 --- a/README.md +++ b/README.md @@ -53,6 +53,8 @@ Run the unit tests via: `npm test` For local headless browser tests run `npm run test-browser` (Requires selenium to be installed - can be done with `npm run selenium-install`) +Running unit tests via `npm test` requires at least node v7.0.0 + ## Browser Testing To run the Selenium tests via Nightwatch serve the app through a local web server: From dc7dc11954a8944177cea0b7b130509e4953e08e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michael=20Fr=C3=B6wis?= Date: Fri, 7 Apr 2017 15:38:53 +0200 Subject: [PATCH 12/18] Static Analysis: library support --- .../modules/checksEffectsInteraction.js | 6 +- .../modules/constantFunctions.js | 8 +- .../modules/staticAnalysisCommon.js | 34 ++++++- .../staticAnalysisCommon-test.js | 96 ++++++++++++++++++- .../staticAnalysisIntegration-test.js | 31 +++--- .../staticanalysis/test-contracts/library.sol | 54 +++++++++++ 6 files changed, 200 insertions(+), 29 deletions(-) create mode 100644 test/staticanalysis/test-contracts/library.sol diff --git a/src/app/staticanalysis/modules/checksEffectsInteraction.js b/src/app/staticanalysis/modules/checksEffectsInteraction.js index 8958c1238f..2544a55e52 100644 --- a/src/app/staticanalysis/modules/checksEffectsInteraction.js +++ b/src/app/staticanalysis/modules/checksEffectsInteraction.js @@ -16,11 +16,10 @@ function checksEffectsInteraction () { checksEffectsInteraction.prototype.report = function (compilationResults) { var warnings = [] + var hasModifiers = this.contracts.some((item) => item.modifiers.length > 0) var cg = callGraph.buildGlobalFuncCallGraph(this.contracts) - 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) => { func.changesState = checkIfChangesState(common.getFullQuallyfiedFuncDefinitionIdent(contract.node, func.node, func.parameters), @@ -30,8 +29,9 @@ checksEffectsInteraction.prototype.report = function (compilationResults) { contract.functions.forEach((func) => { if (isPotentialVulnerableFunction(func, getContext(cg, contract, func))) { var funcName = common.getFullQuallyfiedFuncDefinitionIdent(contract.node, func.node, func.parameters) + var comments = (hasModifiers) ? '
Note:Modifiers are currently not considered by the this static analysis.' : '' warnings.push({ - warning: `Potential Violation of Checks-Effects-Interaction pattern in ${funcName}: Could potentially lead to re-entrancy vulnerability.`, + warning: `Potential Violation of Checks-Effects-Interaction pattern in ${funcName}: Could potentially lead to re-entrancy vulnerability.${comments}`, location: func.src, more: 'http://solidity.readthedocs.io/en/develop/security-considerations.html#re-entrancy' }) diff --git a/src/app/staticanalysis/modules/constantFunctions.js b/src/app/staticanalysis/modules/constantFunctions.js index 2b440ea1b0..cd4159c5a1 100644 --- a/src/app/staticanalysis/modules/constantFunctions.js +++ b/src/app/staticanalysis/modules/constantFunctions.js @@ -16,11 +16,10 @@ function constantFunctions () { constantFunctions.prototype.report = function (compilationResults) { var warnings = [] + var hasModifiers = this.contracts.some((item) => item.modifiers.length > 0) var cg = callGraph.buildGlobalFuncCallGraph(this.contracts) - 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 @@ -32,15 +31,16 @@ constantFunctions.prototype.report = function (compilationResults) { contract.functions.forEach((func) => { if (common.isConstantFunction(func.node) !== func.potentiallyshouldBeConst) { var funcName = common.getFullQuallyfiedFuncDefinitionIdent(contract.node, func.node, func.parameters) + var comments = (hasModifiers) ? '
Note:Modifiers are currently not considered by the this static analysis.' : '' if (func.potentiallyshouldBeConst) { warnings.push({ - warning: `${funcName}: Potentially should be constant but is not.`, + warning: `${funcName}: Potentially should be constant but is not.${comments}`, location: func.src, more: 'http://solidity.readthedocs.io/en/develop/contracts.html#constant-functions' }) } else { warnings.push({ - warning: `${funcName}: Is constant but potentially should not be.`, + warning: `${funcName}: Is constant but potentially should not be.${comments}`, location: func.src, more: 'http://solidity.readthedocs.io/en/develop/contracts.html#constant-functions' }) diff --git a/src/app/staticanalysis/modules/staticAnalysisCommon.js b/src/app/staticanalysis/modules/staticAnalysisCommon.js index 9643bb553c..7877ff8639 100644 --- a/src/app/staticanalysis/modules/staticAnalysisCommon.js +++ b/src/app/staticanalysis/modules/staticAnalysisCommon.js @@ -32,7 +32,8 @@ var basicRegex = { EXTERNALFUNCTIONTYPE: '^function \\(.*\\).* external', CONSTANTFUNCTIONTYPE: '^function \\(.*\\).* constant', REFTYPE: '( storage )|(mapping\\()|(\\[\\])', - FUNCTIONSIGNATURE: '^function \\(([^\\(]*)\\)' + FUNCTIONSIGNATURE: '^function \\(([^\\(]*)\\)', + LIBRARYTYPE: '^type\\(library (.*)\\)' } var basicFunctionTypes = { @@ -77,8 +78,8 @@ function getType (node) { // #################### Complex Getters function getFunctionCallType (func) { - if (!(isExternalDirectCall(func) || isThisLocalCall(func) || isSuperLocalCall(func) || isLocalCall(func))) throw new Error('staticAnalysisCommon.js: not function call Node') - if (isExternalDirectCall(func) || isThisLocalCall(func) || isSuperLocalCall(func)) return func.attributes.type + if (!(isExternalDirectCall(func) || isThisLocalCall(func) || isSuperLocalCall(func) || isLocalCall(func) || isLibraryCall(func))) throw new Error('staticAnalysisCommon.js: not function call Node') + if (isExternalDirectCall(func) || isThisLocalCall(func) || isSuperLocalCall(func) || isLibraryCall(func)) return func.attributes.type return findFirstSubNodeLTR(func, exactMatch(nodeTypes.IDENTIFIER)).attributes.type } @@ -151,11 +152,22 @@ function getFunctionCallTypeParameterType (func) { return new RegExp(basicRegex.FUNCTIONSIGNATURE).exec(getFunctionCallType(func))[1] } +function getLibraryCallContractName (funcCall) { + if (!isLibraryCall(funcCall)) throw new Error('staticAnalysisCommon.js: not an this library call Node') + return new RegExp(basicRegex.LIBRARYTYPE).exec(funcCall.children[0].attributes.type)[1] +} + +function getLibraryCallMemberName (funcCall) { + if (!isLibraryCall(funcCall)) throw new Error('staticAnalysisCommon.js: not an library call Node') + return funcCall.attributes.member_name +} + function getFullQualifiedFunctionCallIdent (contract, func) { if (isLocalCall(func)) return getContractName(contract) + '.' + getLocalCallName(func) + '(' + getFunctionCallTypeParameterType(func) + ')' else if (isThisLocalCall(func)) return getThisLocalCallContractName(func) + '.' + getThisLocalCallName(func) + '(' + getFunctionCallTypeParameterType(func) + ')' else if (isSuperLocalCall(func)) return getContractName(contract) + '.' + getSuperLocalCallName(func) + '(' + getFunctionCallTypeParameterType(func) + ')' else if (isExternalDirectCall(func)) return getExternalDirectCallContractName(func) + '.' + getExternalDirectCallMemberName(func) + '(' + getFunctionCallTypeParameterType(func) + ')' + else if (isLibraryCall(func)) return getLibraryCallContractName(func) + '.' + getLibraryCallMemberName(func) + '(' + getFunctionCallTypeParameterType(func) + ')' else throw new Error('staticAnalysisCommon.js: Can not get function name form non function call node') } @@ -200,7 +212,7 @@ function isInlineAssembly (node) { // #################### Complex Node Identification function isLocalCallGraphRelevantNode (node) { - return ((isLocalCall(node) || isSuperLocalCall(node)) && !isBuiltinFunctionCall(node)) + return ((isLocalCall(node) || isSuperLocalCall(node) || isLibraryCall(node)) && !isBuiltinFunctionCall(node)) } function isBuiltinFunctionCall (node) { @@ -220,7 +232,7 @@ function isEffect (node) { } function isWriteOnStateVariable (effectNode, stateVariables) { - return isEffect(effectNode) && !isInlineAssembly(effectNode) && isStateVariable(getEffectedVariableName(effectNode), stateVariables) + return isInlineAssembly(effectNode) || (isEffect(effectNode) && isStateVariable(getEffectedVariableName(effectNode), stateVariables)) } function isStateVariable (name, stateVariables) { @@ -243,10 +255,18 @@ function isFullyImplementedContract (node) { return nodeType(node, exactMatch(nodeTypes.CONTRACTDEFINITION)) && node.attributes.fullyImplemented === true } +function isLibrary (node) { + return nodeType(node, exactMatch(nodeTypes.CONTRACTDEFINITION)) && node.attributes.isLibrary === true +} + function isCallToNonConstLocalFunction (node) { return isLocalCall(node) && !expressionType(node.children[0], basicRegex.CONSTANTFUNCTIONTYPE) } +function isLibraryCall (node) { + return isMemberAccess(node, basicRegex.FUNCTIONTYPE, undefined, basicRegex.LIBRARYTYPE, undefined) +} + function isExternalDirectCall (node) { return isMemberAccess(node, basicRegex.EXTERNALFUNCTIONTYPE, undefined, basicRegex.CONTRACTTYPE, undefined) && !isThisLocalCall(node) && !isSuperLocalCall(node) } @@ -403,6 +423,8 @@ module.exports = { getExternalDirectCallMemberName: getExternalDirectCallMemberName, getFunctionDefinitionName: getFunctionDefinitionName, getFunctionCallTypeParameterType: getFunctionCallTypeParameterType, + getLibraryCallContractName: getLibraryCallContractName, + getLibraryCallMemberName: getLibraryCallMemberName, getFullQualifiedFunctionCallIdent: getFullQualifiedFunctionCallIdent, getFullQuallyfiedFuncDefinitionIdent: getFullQuallyfiedFuncDefinitionIdent, getStateVariableDeclarationsFormContractNode: getStateVariableDeclarationsFormContractNode, @@ -416,6 +438,7 @@ module.exports = { isBlockBlockHashAccess: isBlockBlockHashAccess, isThisLocalCall: isThisLocalCall, isSuperLocalCall: isSuperLocalCall, + isLibraryCall: isLibraryCall, isLocalCallGraphRelevantNode: isLocalCallGraphRelevantNode, isLocalCall: isLocalCall, isWriteOnStateVariable: isWriteOnStateVariable, @@ -427,6 +450,7 @@ module.exports = { isLowLevelSendInst: isLLSend, isExternalDirectCall: isExternalDirectCall, isFullyImplementedContract: isFullyImplementedContract, + isLibrary: isLibrary, isCallToNonConstLocalFunction: isCallToNonConstLocalFunction, isPlusPlusUnaryOperation: isPlusPlusUnaryOperation, isMinusMinusUnaryOperation: isMinusMinusUnaryOperation, diff --git a/test/staticanalysis/staticAnalysisCommon-test.js b/test/staticanalysis/staticAnalysisCommon-test.js index fe56a6364f..1ae1fde7a5 100644 --- a/test/staticanalysis/staticAnalysisCommon-test.js +++ b/test/staticanalysis/staticAnalysisCommon-test.js @@ -147,7 +147,7 @@ test('staticAnalysisCommon.getType', function (t) { // #################### Complex Getter Test test('staticAnalysisCommon.getFunctionCallType', function (t) { - t.plan(4) + t.plan(5) var localCall = { 'attributes': { 'type': 'tuple()', @@ -198,9 +198,26 @@ test('staticAnalysisCommon.getFunctionCallType', function (t) { name: 'MemberAccess', src: '405:6:0' } - t.ok(common.getFunctionCallType(thisLocalCall) === 'function (bytes32,address) returns (bool)', 'this local call returns correct type') - t.ok(common.getFunctionCallType(externalDirect) === 'function () payable external returns (uint256)', 'external direct call returns correct type') - t.ok(common.getFunctionCallType(localCall) === 'function (struct Ballot.Voter storage pointer)', 'local call returns correct type') + var libCall = { + 'attributes': { + 'member_name': 'insert', + 'type': 'function (struct Set.Data storage pointer,uint256) returns (bool)' + }, + 'children': [ + { + 'attributes': { + 'type': 'type(library Set)', + 'value': 'Set' + }, + 'name': 'Identifier' + } + ], + 'name': 'MemberAccess' + } + t.equal(common.getFunctionCallType(libCall), 'function (struct Set.Data storage pointer,uint256) returns (bool)', 'this lib call returns correct type') + t.equal(common.getFunctionCallType(thisLocalCall), 'function (bytes32,address) returns (bool)', 'this local call returns correct type') + t.equal(common.getFunctionCallType(externalDirect), 'function () payable external returns (uint256)', 'external direct call returns correct type') + t.equal(common.getFunctionCallType(localCall), 'function (struct Ballot.Voter storage pointer)', 'local call returns correct type') t.throws(() => common.getFunctionCallType({ name: 'MemberAccess' }), undefined, 'throws on wrong type') }) @@ -910,6 +927,50 @@ test('staticAnalysisCommon.getFunctionCallTypeParameterType', function (t) { t.throws(() => common.getFunctionCallTypeParameterType({ name: 'MemberAccess' }), undefined, 'throws on wrong type') }) +test('staticAnalysisCommon.getLibraryCallContractName', function (t) { + t.plan(2) + var node = { + 'attributes': { + 'member_name': 'insert', + 'type': 'function (struct Set.Data storage pointer,uint256) returns (bool)' + }, + 'children': [ + { + 'attributes': { + 'type': 'type(library Set)', + 'value': 'Set' + }, + 'name': 'Identifier' + } + ], + 'name': 'MemberAccess' + } + t.equal(common.getLibraryCallContractName(node), 'Set', 'should return correct contract name') + t.throws(() => common.getLibraryCallContractName({ name: 'Identifier' }), undefined, 'should throw on wrong node') +}) + +test('staticAnalysisCommon.getLibraryCallMemberName', function (t) { + t.plan(2) + var node = { + 'attributes': { + 'member_name': 'insert', + 'type': 'function (struct Set.Data storage pointer,uint256) returns (bool)' + }, + 'children': [ + { + 'attributes': { + 'type': 'type(library Set)', + 'value': 'Set' + }, + 'name': 'Identifier' + } + ], + 'name': 'MemberAccess' + } + t.equal(common.getLibraryCallMemberName(node), 'insert', 'should return correct member name') + t.throws(() => common.getLibraryCallMemberName({ name: 'Identifier' }), undefined, 'should throw on wrong node') +}) + test('staticAnalysisCommon.getFullQualifiedFunctionCallIdent', function (t) { t.plan(4) var contract = { name: 'ContractDefinition', attributes: { name: 'baz' } } @@ -1538,7 +1599,7 @@ test('staticAnalysisCommon.isWriteOnStateVariable', function (t) { ], 'name': 'VariableDeclaration' } - t.notOk(common.isWriteOnStateVariable(inlineAssembly, [node1, node2, node3]), 'inline Assembly is not write on state') + t.ok(common.isWriteOnStateVariable(inlineAssembly, [node1, node2, node3]), 'inline Assembly is write on state') t.notOk(common.isWriteOnStateVariable(assignment, [node1, node2, node3]), 'assignment on non state is not write on state') node3.attributes.name = 'c' t.ok(common.isWriteOnStateVariable(assignment, [node1, node2, node3]), 'assignment on state is not write on state') @@ -1777,6 +1838,31 @@ test('staticAnalysisCommon.isSuperLocalCall', function (t) { t.notOk(common.isNowAccess(node), 'is now used should not work') }) +test('staticAnalysisCommon.isLibraryCall', function (t) { + t.plan(5) + var node = { + 'attributes': { + 'member_name': 'insert', + 'type': 'function (struct Set.Data storage pointer,uint256) returns (bool)' + }, + 'children': [ + { + 'attributes': { + 'type': 'type(library Set)', + 'value': 'Set' + }, + 'name': 'Identifier' + } + ], + 'name': 'MemberAccess' + } + t.ok(common.isLibraryCall(node), 'is lib call should not work') + t.notOk(common.isSuperLocalCall(node), 'is super.local_method() used should not work') + 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.notOk(common.isNowAccess(node), 'is now used should not work') +}) + test('staticAnalysisCommon.isLocalCall', function (t) { t.plan(5) var node1 = { diff --git a/test/staticanalysis/staticAnalysisIntegration-test.js b/test/staticanalysis/staticAnalysisIntegration-test.js index 3eba1fb617..a0d7bb5a59 100644 --- a/test/staticanalysis/staticAnalysisIntegration-test.js +++ b/test/staticanalysis/staticAnalysisIntegration-test.js @@ -21,7 +21,8 @@ var testFiles = [ 'notReentrant.sol', 'structReentrant.sol', 'thisLocal.sol', - 'globals.sol' + 'globals.sol', + 'library.sol' ] var testFileAsts = {} @@ -49,7 +50,8 @@ test('Integration test thisLocal.js', function (t) { 'notReentrant.sol': 0, 'structReentrant.sol': 0, 'thisLocal.sol': 1, - 'globals.sol': 0 + 'globals.sol': 0, + 'library.sol': 0 } runModuleOnFiles(module, t, (file, report) => { @@ -64,18 +66,19 @@ test('Integration test checksEffectsInteraction.js', function (t) { var lengthCheck = { 'KingOfTheEtherThrone.sol': 1, - 'assembly.sol': 0, + 'assembly.sol': 1, 'ballot.sol': 0, 'ballot_reentrant.sol': 1, 'ballot_withoutWarnings.sol': 0, 'cross_contract.sol': 0, 'inheritance.sol': 1, - 'modifier1.sol': 1, - 'modifier2.sol': 1, + 'modifier1.sol': 0, + 'modifier2.sol': 0, 'notReentrant.sol': 0, 'structReentrant.sol': 1, 'thisLocal.sol': 0, - 'globals.sol': 1 + 'globals.sol': 1, + 'library.sol': 1 } runModuleOnFiles(module, t, (file, report) => { @@ -96,12 +99,13 @@ test('Integration test constantFunctions.js', function (t) { 'ballot_withoutWarnings.sol': 0, 'cross_contract.sol': 1, 'inheritance.sol': 0, - 'modifier1.sol': 2, - 'modifier2.sol': 1, + 'modifier1.sol': 1, + 'modifier2.sol': 0, 'notReentrant.sol': 0, 'structReentrant.sol': 0, 'thisLocal.sol': 1, - 'globals.sol': 0 + 'globals.sol': 0, + 'library.sol': 1 } runModuleOnFiles(module, t, (file, report) => { @@ -127,7 +131,8 @@ test('Integration test inlineAssembly.js', function (t) { 'notReentrant.sol': 0, 'structReentrant.sol': 0, 'thisLocal.sol': 0, - 'globals.sol': 0 + 'globals.sol': 0, + 'library.sol': 0 } runModuleOnFiles(module, t, (file, report) => { @@ -153,7 +158,8 @@ test('Integration test txOrigin.js', function (t) { 'notReentrant.sol': 0, 'structReentrant.sol': 0, 'thisLocal.sol': 0, - 'globals.sol': 1 + 'globals.sol': 1, + 'library.sol': 0 } runModuleOnFiles(module, t, (file, report) => { @@ -179,7 +185,8 @@ test('Integration test gasCosts.js', function (t) { 'notReentrant.sol': 1, 'structReentrant.sol': 1, 'thisLocal.sol': 2, - 'globals.sol': 1 + 'globals.sol': 1, + 'library.sol': 1 } runModuleOnFiles(module, t, (file, report) => { diff --git a/test/staticanalysis/test-contracts/library.sol b/test/staticanalysis/test-contracts/library.sol new file mode 100644 index 0000000000..3ca1454d49 --- /dev/null +++ b/test/staticanalysis/test-contracts/library.sol @@ -0,0 +1,54 @@ +pragma solidity ^0.4.0; + +library Set { + // We define a new struct datatype that will be used to + // hold its data in the calling contract. + struct Data { mapping(uint => bool) flags; } + + // Note that the first parameter is of type "storage + // reference" and thus only its storage address and not + // its contents is passed as part of the call. This is a + // special feature of library functions. It is idiomatic + // to call the first parameter 'self', if the function can + // be seen as a method of that object. + function insert(Data storage self, uint value) + returns (bool) + { + if (self.flags[value]) + return false; // already there + self.flags[value] = true; + + return true; + } + + function remove(Data storage self, uint value) + returns (bool) + { + if (!self.flags[value]) + return false; // not there + self.flags[value] = false; + return true; + } + + function contains(Data storage self, uint value) + returns (bool) + { + return self.flags[value]; + } +} + + +contract C { + Set.Data knownValues; + + function register(uint value) { + // The library functions can be called without a + // specific instance of the library, since the + // "instance" will be the current contract. + address a; + a.send(10 wei); + if (!Set.insert(knownValues, value)) + throw; + } + // In this contract, we can also directly access knownValues.flags, if we want. +} \ No newline at end of file From 194c59a358fc57b36bdeb91049a4e3f863222dd7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michael=20Fr=C3=B6wis?= Date: Mon, 10 Apr 2017 14:39:21 +0200 Subject: [PATCH 13/18] Static Analysis: us solc/wrapper --- test/staticanalysis/staticAnalysisIntegration-test.js | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/test/staticanalysis/staticAnalysisIntegration-test.js b/test/staticanalysis/staticAnalysisIntegration-test.js index a0d7bb5a59..d0d53e0503 100644 --- a/test/staticanalysis/staticAnalysisIntegration-test.js +++ b/test/staticanalysis/staticAnalysisIntegration-test.js @@ -3,7 +3,8 @@ var test = require('tape') var StatRunner = require('../../src/app/staticanalysis/staticAnalysisRunner') // const util = require('util') -var solc = require('solc') +var solc = require('solc/wrapper') +var compiler = solc(require('../../soljson')) var fs = require('fs') var path = require('path') @@ -29,7 +30,7 @@ var testFileAsts = {} testFiles.forEach((fileName) => { var contents = fs.readFileSync(path.join(__dirname, 'test-contracts', fileName), 'utf8') - testFileAsts[fileName] = solc.compile(contents, 0) + testFileAsts[fileName] = compiler.compile(contents, 0) }) test('Integration test thisLocal.js', function (t) { From 5d2e69c7a8acea33cecf28bc7f9fad60f6b5ac9b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michael=20Fr=C3=B6wis?= Date: Mon, 10 Apr 2017 15:21:16 +0200 Subject: [PATCH 14/18] Static Analysis: fixes testing after merge --- test/staticanalysis/staticAnalysisCommon-test.js | 4 ---- 1 file changed, 4 deletions(-) diff --git a/test/staticanalysis/staticAnalysisCommon-test.js b/test/staticanalysis/staticAnalysisCommon-test.js index 1ae1fde7a5..79c218da74 100644 --- a/test/staticanalysis/staticAnalysisCommon-test.js +++ b/test/staticanalysis/staticAnalysisCommon-test.js @@ -429,9 +429,6 @@ test('staticAnalysisCommon.getSuperLocalCallName', function (t) { ], 'name': 'MemberAccess' } - -test('staticAnalysisCommon.getExternalDirectCallContractName', function (t) { - t.plan(3) var localCall = { 'attributes': { 'type': 'tuple()', @@ -1780,7 +1777,6 @@ test('staticAnalysisCommon.isBlockTimestampAccess', function (t) { t.notOk(common.isNowAccess(node), 'is now used should not work') }) -<<<<<<< 1ca54743848c4b5087105325dbc095c1b6a8b428 test('staticAnalysisCommon.isBlockBlockhashAccess', function (t) { t.plan(4) var node = { From 6bb29630fcd80f0ea1536f30ca5bc4d3f06afb25 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michael=20Fr=C3=B6wis?= Date: Mon, 10 Apr 2017 16:37:49 +0200 Subject: [PATCH 15/18] Static Analysis: fix browser tests --- README.md | 2 +- test-browser/tests/staticanalysis.js | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index 74af5e1d9b..bf0ef7be2d 100644 --- a/README.md +++ b/README.md @@ -64,7 +64,7 @@ To run the Selenium tests via Nightwatch serve the app through a local web serve Then you will need to either: 1. Have a Selenium server running locally on port 4444. - - Run: `npm run browser-test` + - Run: `npm run test-browser` 2. Or, install and run SauceConnect. - Run: `sc -u -k ` (see `.travis.yml` for values) - Run: `npm run browser-test-sc` diff --git a/test-browser/tests/staticanalysis.js b/test-browser/tests/staticanalysis.js index cc5741c593..79155c322c 100644 --- a/test-browser/tests/staticanalysis.js +++ b/test-browser/tests/staticanalysis.js @@ -38,7 +38,7 @@ function runTests (browser) { .click('.staticanalysisView') .click('#staticanalysisView button') .waitForElementPresent('#staticanalysisresult .warning', 2000, true, function () { - dom.listSelectorContains(['Untitled.sol:2:33: use of tx.origin', + dom.listSelectorContains(['Untitled.sol:2:33: Use of tx.origin', 'Fallback function of contract Untitled.sol:TooMuchGas requires too much gas'], '#staticanalysisresult .warning span', browser, function () { From d0e2d1236066b472b0e8ea261a5f780c71742d0a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michael=20Fr=C3=B6wis?= Date: Wed, 10 May 2017 14:11:45 +0200 Subject: [PATCH 16/18] Static Analysis: changes as suggested by chriseth --- .../staticanalysis/modules/abstractAstView.js | 41 ++- .../modules/checksEffectsInteraction.js | 19 +- .../modules/constantFunctions.js | 21 +- .../modules/functionCallGraph.js | 52 +++- src/app/staticanalysis/modules/list.js | 4 +- .../modules/staticAnalysisCommon.js | 294 +++++++++++++++++- 6 files changed, 389 insertions(+), 42 deletions(-) diff --git a/src/app/staticanalysis/modules/abstractAstView.js b/src/app/staticanalysis/modules/abstractAstView.js index 7f2a610a45..20de50aa5a 100644 --- a/src/app/staticanalysis/modules/abstractAstView.js +++ b/src/app/staticanalysis/modules/abstractAstView.js @@ -9,11 +9,36 @@ function abstractAstView () { this.isFunctionNotModifier = false } +/** + * Builds a higher level AST view. I creates a list with each contract as an object in it. + * Example contractsOut: + * + * { + * "node": {}, // actual AST Node of the contract + * "functions": [ + * { + * "node": {}, // actual AST Node of the function + * "relevantNodes": [], // AST nodes in the function that are relevant for the anlysis of this function + * "modifierInvocations": [], // Modifier invocation AST nodes that are applied on this function + * "localVariables": [], // Local variable declaration nodes + * "parameters": [] // Parameter types of the function in order of definition + * } + * ], + * "modifiers": [], // Modifiers definded by the contract, format similar to functions + * "inheritsFrom": [], // Names of contract this one inherits from in order of definition + * "stateVariables": [] // AST nodes of all State variables + * } + * + * @relevantNodeFilter {ASTNode -> bool} function that selects relevant ast nodes for analysis on function level. + * @contractsOut {list} return list for high level AST view + * @return {ASTNode -> void} returns a function that can be used as visit function for static analysis modules, to build up a higher level AST view for further analysis. + */ abstractAstView.prototype.builder = function (relevantNodeFilter, contractsOut) { this.contracts = contractsOut + var that = this return function (node) { if (common.isContractDefinition(node)) { - setCurrentContract(this, { + setCurrentContract(that, { node: node, functions: [], modifiers: [], @@ -21,14 +46,14 @@ abstractAstView.prototype.builder = function (relevantNodeFilter, contractsOut) stateVariables: common.getStateVariableDeclarationsFormContractNode(node) }) } else if (common.isInheritanceSpecifier(node)) { - var currentContract = getCurrentContract(this) + var currentContract = getCurrentContract(that) var inheritsFromName = common.getInheritsFromName(node) currentContract.inheritsFrom.push(inheritsFromName) // add variables from inherited contracts - var inheritsFrom = this.contracts.find((contract) => common.getContractName(contract.node) === inheritsFromName) + var inheritsFrom = that.contracts.find((contract) => common.getContractName(contract.node) === inheritsFromName) currentContract.stateVariables = currentContract.stateVariables.concat(inheritsFrom.stateVariables) } else if (common.isFunctionDefinition(node)) { - setCurrentFunction(this, { + setCurrentFunction(that, { node: node, relevantNodes: [], modifierInvocations: [], @@ -36,17 +61,17 @@ abstractAstView.prototype.builder = function (relevantNodeFilter, contractsOut) parameters: getLocalParameters(node) }) } else if (common.isModifierDefinition(node)) { - setCurrentModifier(this, { + setCurrentModifier(that, { node: node, relevantNodes: [], localVariables: getLocalVariables(node), parameters: getLocalParameters(node) }) } else if (common.isModifierInvocation(node)) { - if (!this.isFunctionNotModifier) throw new Error('abstractAstView.js: Found modifier invocation outside of function scope.') - getCurrentFunction(this).modifierInvocations.push(node) + if (!that.isFunctionNotModifier) throw new Error('abstractAstView.js: Found modifier invocation outside of function scope.') + getCurrentFunction(that).modifierInvocations.push(node) } else if (relevantNodeFilter(node)) { - ((this.isFunctionNotModifier) ? getCurrentFunction(this) : getCurrentModifier(this)).relevantNodes.push(node) + ((that.isFunctionNotModifier) ? getCurrentFunction(that) : getCurrentModifier(that)).relevantNodes.push(node) } } } diff --git a/src/app/staticanalysis/modules/checksEffectsInteraction.js b/src/app/staticanalysis/modules/checksEffectsInteraction.js index 2544a55e52..45c6803f71 100644 --- a/src/app/staticanalysis/modules/checksEffectsInteraction.js +++ b/src/app/staticanalysis/modules/checksEffectsInteraction.js @@ -2,15 +2,16 @@ var name = 'Checks-Effects-Interaction pattern' var desc = 'Avoid potential reentrancy bugs' var categories = require('./categories') var common = require('./staticAnalysisCommon') -var callGraph = require('./functionCallGraph') +var fcallGraph = require('./functionCallGraph') var AbstractAst = require('./abstractAstView') function checksEffectsInteraction () { this.contracts = [] + var that = this checksEffectsInteraction.prototype.visit = new AbstractAst().builder( (node) => common.isInteraction(node) || common.isEffect(node) || common.isLocalCallGraphRelevantNode(node), - this.contracts + that.contracts ) } @@ -18,16 +19,16 @@ checksEffectsInteraction.prototype.report = function (compilationResults) { var warnings = [] var hasModifiers = this.contracts.some((item) => item.modifiers.length > 0) - var cg = callGraph.buildGlobalFuncCallGraph(this.contracts) + var callGraph = fcallGraph.buildGlobalFuncCallGraph(this.contracts) this.contracts.forEach((contract) => { contract.functions.forEach((func) => { func.changesState = checkIfChangesState(common.getFullQuallyfiedFuncDefinitionIdent(contract.node, func.node, func.parameters), - getContext(cg, contract, func)) + getContext(callGraph, contract, func)) }) contract.functions.forEach((func) => { - if (isPotentialVulnerableFunction(func, getContext(cg, contract, func))) { + if (isPotentialVulnerableFunction(func, getContext(callGraph, contract, func))) { var funcName = common.getFullQuallyfiedFuncDefinitionIdent(contract.node, func.node, func.parameters) var comments = (hasModifiers) ? '
Note:Modifiers are currently not considered by the this static analysis.' : '' warnings.push({ @@ -42,8 +43,8 @@ checksEffectsInteraction.prototype.report = function (compilationResults) { return warnings } -function getContext (cg, currentContract, func) { - return { cg: cg, currentContract: currentContract, stateVariables: getStateVariables(currentContract, func) } +function getContext (callGraph, currentContract, func) { + return { callGraph: callGraph, currentContract: currentContract, stateVariables: getStateVariables(currentContract, func) } } function getStateVariables (contract, func) { @@ -65,14 +66,14 @@ function isPotentialVulnerableFunction (func, context) { function isLocalCallWithStateChange (node, context) { if (common.isLocalCallGraphRelevantNode(node)) { - var func = callGraph.resolveCallGraphSymbol(context.cg, common.getFullQualifiedFunctionCallIdent(context.currentContract.node, node)) + var func = fcallGraph.resolveCallGraphSymbol(context.callGraph, common.getFullQualifiedFunctionCallIdent(context.currentContract.node, node)) return !func || (func && func.node.changesState) } return false } function checkIfChangesState (startFuncName, context) { - return callGraph.analyseCallGraph(context.cg, startFuncName, context, (node, context) => common.isWriteOnStateVariable(node, context.stateVariables)) + return fcallGraph.analyseCallGraph(context.callGraph, startFuncName, context, (node, context) => common.isWriteOnStateVariable(node, context.stateVariables)) } module.exports = { diff --git a/src/app/staticanalysis/modules/constantFunctions.js b/src/app/staticanalysis/modules/constantFunctions.js index cd4159c5a1..f5e887a0e3 100644 --- a/src/app/staticanalysis/modules/constantFunctions.js +++ b/src/app/staticanalysis/modules/constantFunctions.js @@ -2,15 +2,16 @@ var name = 'Constant functions' var desc = 'Check for potentially constant functions' var categories = require('./categories') var common = require('./staticAnalysisCommon') -var callGraph = require('./functionCallGraph') +var fcallGraph = require('./functionCallGraph') var AbstractAst = require('./abstractAstView') function constantFunctions () { this.contracts = [] + var that = this constantFunctions.prototype.visit = new AbstractAst().builder( (node) => common.isLowLevelCall(node) || common.isExternalDirectCall(node) || common.isEffect(node) || common.isLocalCallGraphRelevantNode(node) || common.isInlineAssembly(node), - this.contracts + that.contracts ) } @@ -18,17 +19,15 @@ constantFunctions.prototype.report = function (compilationResults) { var warnings = [] var hasModifiers = this.contracts.some((item) => item.modifiers.length > 0) - var cg = callGraph.buildGlobalFuncCallGraph(this.contracts) + var callGraph = fcallGraph.buildGlobalFuncCallGraph(this.contracts) this.contracts.forEach((contract) => { - if (!common.isFullyImplementedContract(contract.node)) return - contract.functions.forEach((func) => { func.potentiallyshouldBeConst = checkIfShouldBeConstant(common.getFullQuallyfiedFuncDefinitionIdent(contract.node, func.node, func.parameters), - getContext(cg, contract, func)) + getContext(callGraph, contract, func)) }) - contract.functions.forEach((func) => { + contract.functions.filter((func) => common.hasFunctionBody(func.node)).forEach((func) => { if (common.isConstantFunction(func.node) !== func.potentiallyshouldBeConst) { var funcName = common.getFullQuallyfiedFuncDefinitionIdent(contract.node, func.node, func.parameters) var comments = (hasModifiers) ? '
Note:Modifiers are currently not considered by the this static analysis.' : '' @@ -52,8 +51,8 @@ constantFunctions.prototype.report = function (compilationResults) { return warnings } -function getContext (cg, currentContract, func) { - return { cg: cg, currentContract: currentContract, stateVariables: getStateVariables(currentContract, func) } +function getContext (callGraph, currentContract, func) { + return { callGraph: callGraph, currentContract: currentContract, stateVariables: getStateVariables(currentContract, func) } } function getStateVariables (contract, func) { @@ -61,7 +60,7 @@ function getStateVariables (contract, func) { } function checkIfShouldBeConstant (startFuncName, context) { - return !callGraph.analyseCallGraph(context.cg, startFuncName, context, isConstBreaker) + return !fcallGraph.analyseCallGraph(context.callGraph, startFuncName, context, isConstBreaker) } function isConstBreaker (node, context) { @@ -74,7 +73,7 @@ function isConstBreaker (node, context) { function isCallOnNonConstExternalInterfaceFunction (node, context) { if (common.isExternalDirectCall(node)) { - var func = callGraph.resolveCallGraphSymbol(context.cg, common.getFullQualifiedFunctionCallIdent(context.currentContract, node)) + var func = fcallGraph.resolveCallGraphSymbol(context.callGraph, common.getFullQualifiedFunctionCallIdent(context.currentContract, node)) return !func || (func && !common.isConstantFunction(func.node.node)) } return false diff --git a/src/app/staticanalysis/modules/functionCallGraph.js b/src/app/staticanalysis/modules/functionCallGraph.js index 6ec502e766..748e2a9b43 100644 --- a/src/app/staticanalysis/modules/functionCallGraph.js +++ b/src/app/staticanalysis/modules/functionCallGraph.js @@ -16,6 +16,30 @@ function buildLocalFuncCallGraphInternal (functions, nodeFilter, extractNodeIden return callGraph } +/** + * Builds a function call graph for the current contracts. + * Example Contract call graph: + * + * { + * "KingOfTheEtherThrone": { + * "contracts": {...}, // Contract node as defined in abstractAstView.js + * "functions": { + * "KingOfTheEtherThrone.claimThrone(string memory)": { // function in KingOfEtherThrone + * "node": {...}, // function node as defined in abstractAstView.js + * "calls": { // list of full qualified function names which are called form this function + * } + * } + * } + * }, + * "foo": { + * "contract": {...}, // Contract node as definded in abstractAstView.js + * "functions": {} // map from full qualified function name to func node + * } + * } + * + * @contracts {list contracts} Expects as input the contract structure defined in abstractAstView.js + * @return {map (string -> Contract Call Graph)} returns map from contract name to contract call graph + */ function buildGlobalFuncCallGraph (contracts) { var callGraph = {} contracts.forEach((contract) => { @@ -29,31 +53,39 @@ function buildGlobalFuncCallGraph (contracts) { return callGraph } -function analyseCallGraph (cg, funcName, context, nodeCheck) { - return analyseCallGraphInternal(cg, funcName, context, (a, b) => a || b, nodeCheck, {}) +/** + * Walks through the call graph from a defined starting function, true if nodeCheck holds for every relevant node in the callgraph + * @callGraph {callGraph} As returned by buildGlobalFuncCallGraph + * @funcName {string} full qualified name of the starting function + * @context {Object} provides additional context information that can be used by the nodeCheck function + * @nodeCheck {(ASTNode, context) -> bool} applied on every relevant node in the call graph + * @return {bool} returns map from contract name to contract call graph + */ +function analyseCallGraph (callGraph, funcName, context, nodeCheck) { + return analyseCallGraphInternal(callGraph, funcName, context, (a, b) => a || b, nodeCheck, {}) } -function analyseCallGraphInternal (cg, funcName, context, combinator, nodeCheck, visited) { - var current = resolveCallGraphSymbol(cg, funcName) +function analyseCallGraphInternal (callGraph, funcName, context, combinator, nodeCheck, visited) { + var current = resolveCallGraphSymbol(callGraph, funcName) if (current === undefined || visited[funcName] === true) return true visited[funcName] = true return combinator(current.node.relevantNodes.reduce((acc, val) => combinator(acc, nodeCheck(val, context)), false), - current.calls.reduce((acc, val) => combinator(acc, analyseCallGraphInternal(cg, val, context, combinator, nodeCheck, visited)), false)) + current.calls.reduce((acc, val) => combinator(acc, analyseCallGraphInternal(callGraph, val, context, combinator, nodeCheck, visited)), false)) } -function resolveCallGraphSymbol (cg, funcName) { - return resolveCallGraphSymbolInternal(cg, funcName, false) +function resolveCallGraphSymbol (callGraph, funcName) { + return resolveCallGraphSymbolInternal(callGraph, funcName, false) } -function resolveCallGraphSymbolInternal (cg, funcName, silent) { +function resolveCallGraphSymbolInternal (callGraph, funcName, silent) { var current if (funcName.includes('.')) { var parts = funcName.split('.') var contractPart = parts[0] var functionPart = parts[1] - var currentContract = cg[contractPart] + var currentContract = callGraph[contractPart] if (!(currentContract === undefined)) { current = currentContract.functions[funcName] // resolve inheritance hierarchy @@ -61,7 +93,7 @@ function resolveCallGraphSymbolInternal (cg, funcName, silent) { // resolve inheritance lookup in linearized fashion var inheritsFromNames = currentContract.contract.inheritsFrom.reverse() for (var i = 0; i < inheritsFromNames.length; i++) { - var res = resolveCallGraphSymbolInternal(cg, inheritsFromNames[i] + '.' + functionPart, true) + var res = resolveCallGraphSymbolInternal(callGraph, inheritsFromNames[i] + '.' + functionPart, true) if (!(res === undefined)) return res } } diff --git a/src/app/staticanalysis/modules/list.js b/src/app/staticanalysis/modules/list.js index a27238f8bb..320189b69b 100644 --- a/src/app/staticanalysis/modules/list.js +++ b/src/app/staticanalysis/modules/list.js @@ -3,8 +3,8 @@ module.exports = [ require('./gasCosts'), require('./thisLocal'), require('./checksEffectsInteraction'), - require('./constantFunctions'), - require('./inlineAssembly') + require('./constantFunctions') + // require('./inlineAssembly'), // require('./blockTimestamp'), // require('./lowLevelCalls'), // require('./blockBlockhash') diff --git a/src/app/staticanalysis/modules/staticAnalysisCommon.js b/src/app/staticanalysis/modules/staticAnalysisCommon.js index 7877ff8639..73d799c1b8 100644 --- a/src/app/staticanalysis/modules/staticAnalysisCommon.js +++ b/src/app/staticanalysis/modules/staticAnalysisCommon.js @@ -16,7 +16,8 @@ var nodeTypes = { MODIFIERINVOCATION: 'ModifierInvocation', INHERITANCESPECIFIER: 'InheritanceSpecifier', USERDEFINEDTYPENAME: 'UserDefinedTypeName', - INLINEASSEMBLY: 'InlineAssembly' + INLINEASSEMBLY: 'InlineAssembly', + BLOCK: 'Block' } var basicTypes = { @@ -50,7 +51,9 @@ var builtinFunctions = { 'ecrecover(bytes32,uint8,bytes32,bytes32)': true, 'addmod(uint256,uint256,uint256)': true, 'mulmod(uint256,uint256,uint256)': true, - 'selfdestruct(address)': true + 'selfdestruct(address)': true, + 'revert()': true, + 'assert()': true } var lowLevelCallTypes = { @@ -77,91 +80,230 @@ function getType (node) { // #################### Complex Getters +/** + * Returns the type parameter of function call AST nodes. Throws if not a function call node + * @func {ASTNode} Function call node + * @return {string} type of function call + */ function getFunctionCallType (func) { if (!(isExternalDirectCall(func) || isThisLocalCall(func) || isSuperLocalCall(func) || isLocalCall(func) || isLibraryCall(func))) throw new Error('staticAnalysisCommon.js: not function call Node') if (isExternalDirectCall(func) || isThisLocalCall(func) || isSuperLocalCall(func) || isLibraryCall(func)) return func.attributes.type return findFirstSubNodeLTR(func, exactMatch(nodeTypes.IDENTIFIER)).attributes.type } +/** + * Get the variable name written to by a effect node, except for inline assembly because there is no information to find out where we write to. Trows if not a effect node or is inlineassmbly. + * Example: x = 10; => x + * @effectNode {ASTNode} Function call node + * @return {string} variable name written to + */ function getEffectedVariableName (effectNode) { if (!isEffect(effectNode) || isInlineAssembly(effectNode)) throw new Error('staticAnalysisCommon.js: not an effect Node or inline assembly') return findFirstSubNodeLTR(effectNode, exactMatch(nodeTypes.IDENTIFIER)).attributes.value } +/** + * Returns the identifier of a local call, Throws on wrong node. + * Example: f(103) => f + * @localCallNode {ASTNode} Function call node + * @return {string} name of the function called + */ function getLocalCallName (localCallNode) { if (!isLocalCall(localCallNode)) throw new Error('staticAnalysisCommon.js: not an local call Node') return localCallNode.children[0].attributes.value } +/** + * Returns the identifier of a this local call, Throws on wrong node. + * Example: this.f(103) => f + * @localCallNode {ASTNode} Function call node + * @return {string} name of the function called + */ function getThisLocalCallName (localCallNode) { if (!isThisLocalCall(localCallNode)) throw new Error('staticAnalysisCommon.js: not an this local call Node') return localCallNode.attributes.value } +/** + * Returns the identifier of a super local call, Throws on wrong node. + * Example: super.f(103) => f + * @localCallNode {ASTNode} Function call node + * @return {string} name of the function called + */ function getSuperLocalCallName (localCallNode) { if (!isSuperLocalCall(localCallNode)) throw new Error('staticAnalysisCommon.js: not an super local call Node') return localCallNode.attributes.member_name } +/** + * Returns the contract type of a external direct call, Throws on wrong node. + * Example: + * foo x = foo(0xdeadbeef...); + * x.f(103) => foo + * @extDirectCall {ASTNode} Function call node + * @return {string} name of the contract the function is defined in + */ function getExternalDirectCallContractName (extDirectCall) { if (!isExternalDirectCall(extDirectCall)) throw new Error('staticAnalysisCommon.js: not an external direct call Node') return extDirectCall.children[0].attributes.type.replace(new RegExp(basicRegex.CONTRACTTYPE), '') } +/** + * Returns the name of the contract of a this local call (current contract), Throws on wrong node. + * Example: + * Contract foo { + * ... + * this.f(103) => foo + * ... + * @thisLocalCall {ASTNode} Function call node + * @return {string} name of the contract the function is defined in + */ function getThisLocalCallContractName (thisLocalCall) { if (!isThisLocalCall(thisLocalCall)) throw new Error('staticAnalysisCommon.js: not an this local call Node') return thisLocalCall.children[0].attributes.type.replace(new RegExp(basicRegex.CONTRACTTYPE), '') } +/** + * Returns the function identifier of a external direct call, Throws on wrong node. + * Example: + * foo x = foo(0xdeadbeef...); + * x.f(103) => f + * @extDirectCall {ASTNode} Function call node + * @return {string} name of the function called + */ function getExternalDirectCallMemberName (extDirectCall) { if (!isExternalDirectCall(extDirectCall)) throw new Error('staticAnalysisCommon.js: not an external direct call Node') return extDirectCall.attributes.member_name } +/** + * Returns the name of a contract, Throws on wrong node. + * Example: + * Contract foo { => foo + * @contract {ASTNode} Contract Definition node + * @return {string} name of a contract defined + */ function getContractName (contract) { if (!isContractDefinition(contract)) throw new Error('staticAnalysisCommon.js: not an contractDefinition Node') return contract.attributes.name } +/** + * Returns the name of a function definition, Throws on wrong node. + * Example: + * func foo(uint bla) { => foo + * @funcDef {ASTNode} Function Definition node + * @return {string} name of a function defined + */ function getFunctionDefinitionName (funcDef) { if (!isFunctionDefinition(funcDef)) throw new Error('staticAnalysisCommon.js: not an functionDefinition Node') return funcDef.attributes.name } +/** + * Returns the identifier of an inheritance specifier, Throws on wrong node. + * Example: + * contract KingOfTheEtherThrone is b { => b + * @func {ASTNode} Inheritance specifier + * @return {string} name of contract inherited from + */ function getInheritsFromName (inheritsNode) { if (!isInheritanceSpecifier(inheritsNode)) throw new Error('staticAnalysisCommon.js: not an InheritanceSpecifier node Node') return inheritsNode.children[0].attributes.name } +/** + * Returns the identifier of a variable definition, Throws on wrong node. + * Example: + * var x = 10; => x + * @varDeclNode {ASTNode} Variable declaration node + * @return {string} variable name + */ function getDeclaredVariableName (varDeclNode) { if (!isVariableDeclaration(varDeclNode)) throw new Error('staticAnalysisCommon.js: not an variable declaration') return varDeclNode.attributes.name } +/** + * Returns state variable declaration nodes for a contract, Throws on wrong node. + * Example: + * contract foo { + * ... + * var y = true; + * var x = 10; => [y,x] + * @contractNode {ASTNode} Contract Definition node + * @return {list variable declaration} state variable node list + */ function getStateVariableDeclarationsFormContractNode (contractNode) { if (!isContractDefinition(contractNode)) throw new Error('staticAnalysisCommon.js: not an contract definition declaration') return contractNode.children.filter((el) => isVariableDeclaration(el)) } +/** + * Returns parameter node for a function or modifier definition, Throws on wrong node. + * Example: + * function bar(uint a, uint b) => uint a, uint b + * @funcNode {ASTNode} Contract Definition node + * @return {parameterlist node} parameterlist node + */ function getFunctionOrModifierDefinitionParameterPart (funcNode) { if (!isFunctionDefinition(funcNode) && !isModifierDefinition(funcNode)) throw new Error('staticAnalysisCommon.js: not an function definition') return funcNode.children[0] } +/** + * Extracts the parameter types for a function type signature + * Example: + * function(uint a, uint b) returns (bool) => uint a, uint b + * @func {ASTNode} function call node + * @return {string} parameter signature + */ function getFunctionCallTypeParameterType (func) { return new RegExp(basicRegex.FUNCTIONSIGNATURE).exec(getFunctionCallType(func))[1] } +/** + * Returns the name of the library called, Throws on wrong node. + * Example: + * library set{...} + * contract foo { + * ... + * function () { set.union() => set} + * @funcCall {ASTNode} function call node + * @return {string} name of the lib defined + */ function getLibraryCallContractName (funcCall) { if (!isLibraryCall(funcCall)) throw new Error('staticAnalysisCommon.js: not an this library call Node') return new RegExp(basicRegex.LIBRARYTYPE).exec(funcCall.children[0].attributes.type)[1] } +/** + * Returns the name of the function of a library call, Throws on wrong node. + * Example: + * library set{...} + * contract foo { + * ... + * function () { set.union() => uinion} + * @func {ASTNode} function call node + * @return {string} name of function called on the library + */ function getLibraryCallMemberName (funcCall) { if (!isLibraryCall(funcCall)) throw new Error('staticAnalysisCommon.js: not an library call Node') return funcCall.attributes.member_name } +/** + * Returns full qualified name for a function call, Throws on wrong node. + * Example: + * contract foo { + * ... + * function bar(uint b) { } + * function baz() { + * bar(10) => foo.bar(uint) + * @func {ASTNode} function call node + * @func {ASTNode} contract defintion + * @return {string} full qualified identifier for the function call + */ function getFullQualifiedFunctionCallIdent (contract, func) { if (isLocalCall(func)) return getContractName(contract) + '.' + getLocalCallName(func) + '(' + getFunctionCallTypeParameterType(func) + ')' else if (isThisLocalCall(func)) return getThisLocalCallContractName(func) + '.' + getThisLocalCallName(func) + '(' + getFunctionCallTypeParameterType(func) + ')' @@ -211,88 +353,204 @@ function isInlineAssembly (node) { // #################### Complex Node Identification +/** + * True if function defintion has function body + * @funcNode {ASTNode} function defintion node + * @return {bool} + */ +function hasFunctionBody (funcNode) { + return findFirstSubNodeLTR(funcNode, exactMatch(nodeTypes.BLOCK)) != null +} + +/** + * True if call to code within the current contracts context including (delegate) library call + * @node {ASTNode} some AstNode + * @return {bool} + */ function isLocalCallGraphRelevantNode (node) { return ((isLocalCall(node) || isSuperLocalCall(node) || isLibraryCall(node)) && !isBuiltinFunctionCall(node)) } +/** + * True if is builtin function like assert, sha3, erecover, ... + * @node {ASTNode} some AstNode + * @return {bool} + */ function isBuiltinFunctionCall (node) { return isLocalCall(node) && builtinFunctions[getLocalCallName(node) + '(' + getFunctionCallTypeParameterType(node) + ')'] === true } +/** + * True if is storage variable declaration + * @node {ASTNode} some AstNode + * @return {bool} + */ function isStorageVariableDeclaration (node) { return isVariableDeclaration(node) && expressionType(node, basicRegex.REFTYPE) } +/** + * True if is interaction with external contract (change in context, no delegate calls) (send, call of other contracts) + * @node {ASTNode} some AstNode + * @return {bool} + */ function isInteraction (node) { return isLLCall(node) || isLLSend(node) || isExternalDirectCall(node) } +/** + * True if node changes state of a variable or is inline assembly (does not include check if it is a global state change, on a state variable) + * @node {ASTNode} some AstNode + * @return {bool} + */ function isEffect (node) { return isAssignment(node) || isPlusPlusUnaryOperation(node) || isMinusMinusUnaryOperation(node) || isInlineAssembly(node) } +/** + * True if node changes state of a variable or is inline assembly (Checks if variable is a state variable via provided list) + * @node {ASTNode} some AstNode + * @node {list Variable declaration} state variable declaration currently in scope + * @return {bool} + */ function isWriteOnStateVariable (effectNode, stateVariables) { return isInlineAssembly(effectNode) || (isEffect(effectNode) && isStateVariable(getEffectedVariableName(effectNode), stateVariables)) } +/** + * True if there is a variable with name, name in stateVariables + * @node {ASTNode} some AstNode + * @node {list Variable declaration} state variable declaration currently in scope + * @return {bool} + */ function isStateVariable (name, stateVariables) { return stateVariables.some((item) => name === getDeclaredVariableName(item)) } +/** + * True if is function defintion that is flaged as constant + * @node {ASTNode} some AstNode + * @return {bool} + */ function isConstantFunction (node) { return isFunctionDefinition(node) && node.attributes.constant === true } +/** + * True if unary increment operation + * @node {ASTNode} some AstNode + * @return {bool} + */ function isPlusPlusUnaryOperation (node) { return nodeType(node, exactMatch(nodeTypes.UNARYOPERATION)) && operator(node, exactMatch(utils.escapeRegExp('++'))) } +/** + * True if unary decrement operation + * @node {ASTNode} some AstNode + * @return {bool} + */ function isMinusMinusUnaryOperation (node) { return nodeType(node, exactMatch(nodeTypes.UNARYOPERATION)) && operator(node, exactMatch(utils.escapeRegExp('--'))) } +/** + * True if all functions on a contract are implemented + * @node {ASTNode} some AstNode + * @return {bool} + */ function isFullyImplementedContract (node) { return nodeType(node, exactMatch(nodeTypes.CONTRACTDEFINITION)) && node.attributes.fullyImplemented === true } +/** + * True if it is a library contract defintion + * @node {ASTNode} some AstNode + * @return {bool} + */ function isLibrary (node) { return nodeType(node, exactMatch(nodeTypes.CONTRACTDEFINITION)) && node.attributes.isLibrary === true } +/** + * True if it is a local call to non const function + * @node {ASTNode} some AstNode + * @return {bool} + */ function isCallToNonConstLocalFunction (node) { return isLocalCall(node) && !expressionType(node.children[0], basicRegex.CONSTANTFUNCTIONTYPE) } +/** + * True if it is a call to a library + * @node {ASTNode} some AstNode + * @return {bool} + */ function isLibraryCall (node) { return isMemberAccess(node, basicRegex.FUNCTIONTYPE, undefined, basicRegex.LIBRARYTYPE, undefined) } +/** + * True if it is an external call via defined interface (not low level call) + * @node {ASTNode} some AstNode + * @return {bool} + */ function isExternalDirectCall (node) { return isMemberAccess(node, basicRegex.EXTERNALFUNCTIONTYPE, undefined, basicRegex.CONTRACTTYPE, undefined) && !isThisLocalCall(node) && !isSuperLocalCall(node) } +/** + * True if access to block.timestamp via now alias + * @node {ASTNode} some AstNode + * @return {bool} + */ function isNowAccess (node) { return nodeType(node, exactMatch(nodeTypes.IDENTIFIER)) && expressionType(node, exactMatch(basicTypes.UINT)) && name(node, exactMatch('now')) } +/** + * True if access to block.timestamp + * @node {ASTNode} some AstNode + * @return {bool} + */ function isBlockTimestampAccess (node) { return isSpecialVariableAccess(node, specialVariables.BLOCKTIMESTAMP) } +/** + * True if access to block.blockhash + * @node {ASTNode} some AstNode + * @return {bool} + */ function isBlockBlockHashAccess (node) { return isSpecialVariableAccess(node, specialVariables.BLOCKHASH) } +/** + * True if call to local function via this keyword + * @node {ASTNode} some AstNode + * @return {bool} + */ function isThisLocalCall (node) { return isMemberAccess(node, basicRegex.FUNCTIONTYPE, exactMatch('this'), basicRegex.CONTRACTTYPE, undefined) } +/** + * True if access to local function via super keyword + * @node {ASTNode} some AstNode + * @return {bool} + */ function isSuperLocalCall (node) { return isMemberAccess(node, basicRegex.FUNCTIONTYPE, exactMatch('super'), basicRegex.CONTRACTTYPE, undefined) } +/** + * True if call to local function + * @node {ASTNode} some AstNode + * @return {bool} + */ function isLocalCall (node) { return nodeType(node, exactMatch(nodeTypes.FUNCTIONCALL)) && minNrOfChildren(node, 1) && @@ -302,6 +560,11 @@ function isLocalCall (node) { nrOfChildren(node.children[0], 0) } +/** + * True if low level call (send, call, delegatecall, callcode) + * @node {ASTNode} some AstNode + * @return {bool} + */ function isLowLevelCall (node) { return isLLCall(node) || isLLCallcode(node) || @@ -309,24 +572,44 @@ function isLowLevelCall (node) { isLLSend(node) } +/** + * True if low level send + * @node {ASTNode} some AstNode + * @return {bool} + */ function isLLSend (node) { return isMemberAccess(node, exactMatch(utils.escapeRegExp(lowLevelCallTypes.SEND.type)), undefined, exactMatch(basicTypes.ADDRESS), exactMatch(lowLevelCallTypes.SEND.ident)) } +/** + * True if low level call + * @node {ASTNode} some AstNode + * @return {bool} + */ function isLLCall (node) { return isMemberAccess(node, exactMatch(utils.escapeRegExp(lowLevelCallTypes.CALL.type)), undefined, exactMatch(basicTypes.ADDRESS), exactMatch(lowLevelCallTypes.CALL.ident)) } +/** + * True if low level callcode + * @node {ASTNode} some AstNode + * @return {bool} + */ function isLLCallcode (node) { return isMemberAccess(node, exactMatch(utils.escapeRegExp(lowLevelCallTypes.CALLCODE.type)), undefined, exactMatch(basicTypes.ADDRESS), exactMatch(lowLevelCallTypes.CALLCODE.ident)) } +/** + * True if low level delegatecall + * @node {ASTNode} some AstNode + * @return {bool} + */ function isLLDelegatecall (node) { return isMemberAccess(node, exactMatch(utils.escapeRegExp(lowLevelCallTypes.DELEGATECALL.type)), @@ -393,6 +676,12 @@ function buildFunctionSignature (paramTypes, returnTypes, isPayable) { return 'function (' + utils.concatWithSeperator(paramTypes, ',') + ')' + ((isPayable) ? ' payable' : '') + ((returnTypes.length) ? ' returns (' + utils.concatWithSeperator(returnTypes, ',') + ')' : '') } +/** + * Finds first node of a certain type under a specific node. + * @node {AstNode} node to start form + * @type {String} Type the ast node should have + * @return {AstNode} null or node found + */ function findFirstSubNodeLTR (node, type) { if (!node || !node.children) return null for (let i = 0; i < node.children.length; ++i) { @@ -431,6 +720,7 @@ module.exports = { getFunctionOrModifierDefinitionParameterPart: getFunctionOrModifierDefinitionParameterPart, // #################### Complex Node Identification + hasFunctionBody: hasFunctionBody, isInteraction: isInteraction, isEffect: isEffect, isNowAccess: isNowAccess, From 85ae6115e5e9b00e845fcc0272aa81c9d9f3ade5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michael=20Fr=C3=B6wis?= Date: Wed, 17 May 2017 10:46:24 +0200 Subject: [PATCH 17/18] Static Analysis: new as const breaker --- .../modules/constantFunctions.js | 6 +++-- .../modules/staticAnalysisCommon.js | 26 ++++++++++++++++--- 2 files changed, 27 insertions(+), 5 deletions(-) diff --git a/src/app/staticanalysis/modules/constantFunctions.js b/src/app/staticanalysis/modules/constantFunctions.js index f5e887a0e3..ac76a90cd9 100644 --- a/src/app/staticanalysis/modules/constantFunctions.js +++ b/src/app/staticanalysis/modules/constantFunctions.js @@ -10,7 +10,7 @@ function constantFunctions () { var that = this constantFunctions.prototype.visit = new AbstractAst().builder( - (node) => common.isLowLevelCall(node) || common.isExternalDirectCall(node) || common.isEffect(node) || common.isLocalCallGraphRelevantNode(node) || common.isInlineAssembly(node), + (node) => common.isLowLevelCall(node) || common.isTransfer(node) || common.isExternalDirectCall(node) || common.isEffect(node) || common.isLocalCallGraphRelevantNode(node) || common.isInlineAssembly(node) || common.isNewExpression(node), that.contracts ) } @@ -66,9 +66,11 @@ function checkIfShouldBeConstant (startFuncName, context) { function isConstBreaker (node, context) { return common.isWriteOnStateVariable(node, context.stateVariables) || common.isLowLevelCall(node) || + common.isTransfer(node) || isCallOnNonConstExternalInterfaceFunction(node, context) || common.isCallToNonConstLocalFunction(node) || - common.isInlineAssembly(node) + common.isInlineAssembly(node) || + common.isNewExpression(node) } function isCallOnNonConstExternalInterfaceFunction (node, context) { diff --git a/src/app/staticanalysis/modules/staticAnalysisCommon.js b/src/app/staticanalysis/modules/staticAnalysisCommon.js index 73d799c1b8..36c3014599 100644 --- a/src/app/staticanalysis/modules/staticAnalysisCommon.js +++ b/src/app/staticanalysis/modules/staticAnalysisCommon.js @@ -17,7 +17,8 @@ var nodeTypes = { INHERITANCESPECIFIER: 'InheritanceSpecifier', USERDEFINEDTYPENAME: 'UserDefinedTypeName', INLINEASSEMBLY: 'InlineAssembly', - BLOCK: 'Block' + BLOCK: 'Block', + NEWEXPRESSION: 'NewExpression' } var basicTypes = { @@ -40,7 +41,8 @@ var basicRegex = { var basicFunctionTypes = { SEND: buildFunctionSignature([basicTypes.UINT], [basicTypes.BOOL], false), CALL: buildFunctionSignature([], [basicTypes.BOOL], true), - DELEGATECALL: buildFunctionSignature([], [basicTypes.BOOL], false) + DELEGATECALL: buildFunctionSignature([], [basicTypes.BOOL], false), + TRANSFER: buildFunctionSignature([basicTypes.UINT], [], false) } var builtinFunctions = { @@ -60,7 +62,8 @@ var lowLevelCallTypes = { CALL: { ident: 'call', type: basicFunctionTypes.CALL }, CALLCODE: { ident: 'callcode', type: basicFunctionTypes.CALL }, DELEGATECALL: { ident: 'delegatecall', type: basicFunctionTypes.DELEGATECALL }, - SEND: { ident: 'send', type: basicFunctionTypes.SEND } + SEND: { ident: 'send', type: basicFunctionTypes.SEND }, + TRANSFER: { ident: 'transfer', type: basicFunctionTypes.TRANSFER } } var specialVariables = { @@ -351,6 +354,10 @@ function isInlineAssembly (node) { return nodeType(node, exactMatch(nodeTypes.INLINEASSEMBLY)) } +function isNewExpression (node) { + return nodeType(node, exactMatch(nodeTypes.NEWEXPRESSION)) +} + // #################### Complex Node Identification /** @@ -616,6 +623,17 @@ function isLLDelegatecall (node) { undefined, exactMatch(basicTypes.ADDRESS), exactMatch(lowLevelCallTypes.DELEGATECALL.ident)) } +/** + * True if transfer call + * @node {ASTNode} some AstNode + * @return {bool} + */ +function isTransfer (node) { + return isMemberAccess(node, + exactMatch(utils.escapeRegExp(lowLevelCallTypes.TRANSFER.type)), + undefined, exactMatch(basicTypes.ADDRESS), exactMatch(lowLevelCallTypes.TRANSFER.ident)) +} + // #################### Complex Node Identification - Private function isMemberAccess (node, retType, accessor, accessorType, memberName) { @@ -733,6 +751,7 @@ module.exports = { isLocalCall: isLocalCall, isWriteOnStateVariable: isWriteOnStateVariable, isStateVariable: isStateVariable, + isTransfer: isTransfer, isLowLevelCall: isLowLevelCall, isLowLevelCallInst: isLLCall, isLowLevelCallcodeInst: isLLCallcode, @@ -757,6 +776,7 @@ module.exports = { isContractDefinition: isContractDefinition, isConstantFunction: isConstantFunction, isInlineAssembly: isInlineAssembly, + isNewExpression: isNewExpression, // #################### Constants nodeTypes: nodeTypes, From 7ef1afc3313618caa0bf4a920167eecdc88e9d78 Mon Sep 17 00:00:00 2001 From: soad003 Date: Wed, 24 May 2017 15:54:41 +0200 Subject: [PATCH 18/18] Static Analysis: improvements suggested by chriseth --- .../staticanalysis/modules/abstractAstView.js | 46 ++++++++++++++++--- .../modules/checksEffectsInteraction.js | 28 ++++++----- .../modules/constantFunctions.js | 29 +++++++----- src/app/staticanalysis/modules/list.js | 1 + .../modules/staticAnalysisCommon.js | 3 +- 5 files changed, 76 insertions(+), 31 deletions(-) diff --git a/src/app/staticanalysis/modules/abstractAstView.js b/src/app/staticanalysis/modules/abstractAstView.js index 20de50aa5a..0a5fc80831 100644 --- a/src/app/staticanalysis/modules/abstractAstView.js +++ b/src/app/staticanalysis/modules/abstractAstView.js @@ -2,11 +2,20 @@ var common = require('./staticAnalysisCommon') var AstWalker = require('ethereum-remix').util.AstWalker function abstractAstView () { - this.contracts = null + this.contracts = [] this.currentContractIndex = null this.currentFunctionIndex = null this.currentModifierIndex = null this.isFunctionNotModifier = false + /* + file1: contract c{} + file2: import "file1" as x; contract c{} + therefore we have two contracts with the same name c. At the moment this is not handled because alias name "x" is not + available in the current AST implementation thus can not be resolved. + Additionally the fullQuallified function names e.g. [contractName].[functionName](param1Type, param2Type, ... ) must be prefixed to + fully support this and when inheritance is resolved it must include alias resolving e.g x.c = file1.c + */ + this.multipleContractsWithSameName = false } /** @@ -33,8 +42,7 @@ function abstractAstView () { * @contractsOut {list} return list for high level AST view * @return {ASTNode -> void} returns a function that can be used as visit function for static analysis modules, to build up a higher level AST view for further analysis. */ -abstractAstView.prototype.builder = function (relevantNodeFilter, contractsOut) { - this.contracts = contractsOut +abstractAstView.prototype.build_visit = function (relevantNodeFilter) { var that = this return function (node) { if (common.isContractDefinition(node)) { @@ -49,9 +57,6 @@ abstractAstView.prototype.builder = function (relevantNodeFilter, contractsOut) var currentContract = getCurrentContract(that) var inheritsFromName = common.getInheritsFromName(node) currentContract.inheritsFrom.push(inheritsFromName) - // add variables from inherited contracts - var inheritsFrom = that.contracts.find((contract) => common.getContractName(contract.node) === inheritsFromName) - currentContract.stateVariables = currentContract.stateVariables.concat(inheritsFrom.stateVariables) } else if (common.isFunctionDefinition(node)) { setCurrentFunction(that, { node: node, @@ -76,7 +81,36 @@ abstractAstView.prototype.builder = function (relevantNodeFilter, contractsOut) } } +abstractAstView.prototype.build_report = function (wrap) { + var that = this + return function (compilationResult) { + resolveStateVariablesInHierarchy(that.contracts) + return wrap(that.contracts, that.multipleContractsWithSameName) + } +} + +function resolveStateVariablesInHierarchy (contracts) { + contracts.map((c) => { + resolveStateVariablesInHierarchyForContract(c, contracts) + }) +} +function resolveStateVariablesInHierarchyForContract (currentContract, contracts) { + currentContract.inheritsFrom.map((inheritsFromName) => { + // add variables from inherited contracts + var inheritsFrom = contracts.find((contract) => common.getContractName(contract.node) === inheritsFromName) + if (inheritsFrom) { + currentContract.stateVariables = currentContract.stateVariables.concat(inheritsFrom.stateVariables) + } else { + console.log('abstractAstView.js: could not find contract defintion inherited from ' + inheritsFromName) + } + }) +} function setCurrentContract (that, contract) { + var name = common.getContractName(contract.node) + if (that.contracts.map((c) => common.getContractName(c.node)).filter((n) => n === name).length > 0) { + console.log('abstractAstView.js: two or more contracts with the same name dectected, import aliases not supported at the moment') + that.multipleContractsWithSameName = true + } that.currentContractIndex = (that.contracts.push(contract) - 1) } diff --git a/src/app/staticanalysis/modules/checksEffectsInteraction.js b/src/app/staticanalysis/modules/checksEffectsInteraction.js index 45c6803f71..b759d242eb 100644 --- a/src/app/staticanalysis/modules/checksEffectsInteraction.js +++ b/src/app/staticanalysis/modules/checksEffectsInteraction.js @@ -6,22 +6,25 @@ var fcallGraph = require('./functionCallGraph') var AbstractAst = require('./abstractAstView') function checksEffectsInteraction () { - this.contracts = [] - var that = this - - checksEffectsInteraction.prototype.visit = new AbstractAst().builder( - (node) => common.isInteraction(node) || common.isEffect(node) || common.isLocalCallGraphRelevantNode(node), - that.contracts + this.abstractAst = new AbstractAst() + this.visit = this.abstractAst.build_visit( + (node) => common.isInteraction(node) || common.isEffect(node) || common.isLocalCallGraphRelevantNode(node) ) + + this.report = this.abstractAst.build_report(report) } -checksEffectsInteraction.prototype.report = function (compilationResults) { +checksEffectsInteraction.prototype.visit = function () { throw new Error('checksEffectsInteraction.js no visit function set upon construction') } + +checksEffectsInteraction.prototype.report = function () { throw new Error('checksEffectsInteraction.js no report function set upon construction') } + +function report (contracts, multipleContractsWithSameName) { var warnings = [] - var hasModifiers = this.contracts.some((item) => item.modifiers.length > 0) + var hasModifiers = contracts.some((item) => item.modifiers.length > 0) - var callGraph = fcallGraph.buildGlobalFuncCallGraph(this.contracts) + var callGraph = fcallGraph.buildGlobalFuncCallGraph(contracts) - this.contracts.forEach((contract) => { + contracts.forEach((contract) => { contract.functions.forEach((func) => { func.changesState = checkIfChangesState(common.getFullQuallyfiedFuncDefinitionIdent(contract.node, func.node, func.parameters), getContext(callGraph, contract, func)) @@ -30,9 +33,10 @@ checksEffectsInteraction.prototype.report = function (compilationResults) { contract.functions.forEach((func) => { if (isPotentialVulnerableFunction(func, getContext(callGraph, contract, func))) { var funcName = common.getFullQuallyfiedFuncDefinitionIdent(contract.node, func.node, func.parameters) - var comments = (hasModifiers) ? '
Note:Modifiers are currently not considered by the this static analysis.' : '' + var comments = (hasModifiers) ? '
Note: Modifiers are currently not considered by this static analysis.' : '' + comments += (multipleContractsWithSameName) ? '
Note: Import aliases are currently not supported by this static analysis.' : '' warnings.push({ - warning: `Potential Violation of Checks-Effects-Interaction pattern in ${funcName}: Could potentially lead to re-entrancy vulnerability.${comments}`, + warning: `Potential Violation of Checks-Effects-Interaction pattern in ${funcName}: Could potentially lead to re-entrancy vulnerability. ${comments}`, location: func.src, more: 'http://solidity.readthedocs.io/en/develop/security-considerations.html#re-entrancy' }) diff --git a/src/app/staticanalysis/modules/constantFunctions.js b/src/app/staticanalysis/modules/constantFunctions.js index ac76a90cd9..59988606e0 100644 --- a/src/app/staticanalysis/modules/constantFunctions.js +++ b/src/app/staticanalysis/modules/constantFunctions.js @@ -6,22 +6,26 @@ var fcallGraph = require('./functionCallGraph') var AbstractAst = require('./abstractAstView') function constantFunctions () { - this.contracts = [] - var that = this + this.abstractAst = new AbstractAst() - constantFunctions.prototype.visit = new AbstractAst().builder( - (node) => common.isLowLevelCall(node) || common.isTransfer(node) || common.isExternalDirectCall(node) || common.isEffect(node) || common.isLocalCallGraphRelevantNode(node) || common.isInlineAssembly(node) || common.isNewExpression(node), - that.contracts + this.visit = this.abstractAst.build_visit( + (node) => common.isLowLevelCall(node) || common.isTransfer(node) || common.isExternalDirectCall(node) || common.isEffect(node) || common.isLocalCallGraphRelevantNode(node) || common.isInlineAssembly(node) || common.isNewExpression(node) ) + + this.report = this.abstractAst.build_report(report) } -constantFunctions.prototype.report = function (compilationResults) { +constantFunctions.prototype.visit = function () { throw new Error('constantFunctions.js no visit function set upon construction') } + +constantFunctions.prototype.report = function () { throw new Error('constantFunctions.js no report function set upon construction') } + +function report (contracts, multipleContractsWithSameName) { var warnings = [] - var hasModifiers = this.contracts.some((item) => item.modifiers.length > 0) + var hasModifiers = contracts.some((item) => item.modifiers.length > 0) - var callGraph = fcallGraph.buildGlobalFuncCallGraph(this.contracts) + var callGraph = fcallGraph.buildGlobalFuncCallGraph(contracts) - this.contracts.forEach((contract) => { + contracts.forEach((contract) => { contract.functions.forEach((func) => { func.potentiallyshouldBeConst = checkIfShouldBeConstant(common.getFullQuallyfiedFuncDefinitionIdent(contract.node, func.node, func.parameters), getContext(callGraph, contract, func)) @@ -30,16 +34,17 @@ constantFunctions.prototype.report = function (compilationResults) { contract.functions.filter((func) => common.hasFunctionBody(func.node)).forEach((func) => { if (common.isConstantFunction(func.node) !== func.potentiallyshouldBeConst) { var funcName = common.getFullQuallyfiedFuncDefinitionIdent(contract.node, func.node, func.parameters) - var comments = (hasModifiers) ? '
Note:Modifiers are currently not considered by the this static analysis.' : '' + var comments = (hasModifiers) ? '
Note: Modifiers are currently not considered by this static analysis.' : '' + comments += (multipleContractsWithSameName) ? '
Note: Import aliases are currently not supported by this static analysis.' : '' if (func.potentiallyshouldBeConst) { warnings.push({ - warning: `${funcName}: Potentially should be constant but is not.${comments}`, + warning: `${funcName}: Potentially should be constant but is not. ${comments}`, location: func.src, more: 'http://solidity.readthedocs.io/en/develop/contracts.html#constant-functions' }) } else { warnings.push({ - warning: `${funcName}: Is constant but potentially should not be.${comments}`, + warning: `${funcName}: Is constant but potentially should not be. ${comments}`, location: func.src, more: 'http://solidity.readthedocs.io/en/develop/contracts.html#constant-functions' }) diff --git a/src/app/staticanalysis/modules/list.js b/src/app/staticanalysis/modules/list.js index 320189b69b..cd17ab43cb 100644 --- a/src/app/staticanalysis/modules/list.js +++ b/src/app/staticanalysis/modules/list.js @@ -4,6 +4,7 @@ module.exports = [ require('./thisLocal'), require('./checksEffectsInteraction'), require('./constantFunctions') + // require('./similarVariableNames.js'), // require('./inlineAssembly'), // require('./blockTimestamp'), // require('./lowLevelCalls'), diff --git a/src/app/staticanalysis/modules/staticAnalysisCommon.js b/src/app/staticanalysis/modules/staticAnalysisCommon.js index 36c3014599..2876644005 100644 --- a/src/app/staticanalysis/modules/staticAnalysisCommon.js +++ b/src/app/staticanalysis/modules/staticAnalysisCommon.js @@ -55,7 +55,8 @@ var builtinFunctions = { 'mulmod(uint256,uint256,uint256)': true, 'selfdestruct(address)': true, 'revert()': true, - 'assert()': true + 'assert(bool)': true, + 'require(bool)': true } var lowLevelCallTypes = {