Commit d0e2d123 authored by Michael Fröwis's avatar Michael Fröwis Committed by chriseth

Static Analysis: changes as suggested by chriseth

parent 6bb29630
...@@ -9,11 +9,36 @@ function abstractAstView () { ...@@ -9,11 +9,36 @@ function abstractAstView () {
this.isFunctionNotModifier = false 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) { abstractAstView.prototype.builder = function (relevantNodeFilter, contractsOut) {
this.contracts = contractsOut this.contracts = contractsOut
var that = this
return function (node) { return function (node) {
if (common.isContractDefinition(node)) { if (common.isContractDefinition(node)) {
setCurrentContract(this, { setCurrentContract(that, {
node: node, node: node,
functions: [], functions: [],
modifiers: [], modifiers: [],
...@@ -21,14 +46,14 @@ abstractAstView.prototype.builder = function (relevantNodeFilter, contractsOut) ...@@ -21,14 +46,14 @@ abstractAstView.prototype.builder = function (relevantNodeFilter, contractsOut)
stateVariables: common.getStateVariableDeclarationsFormContractNode(node) stateVariables: common.getStateVariableDeclarationsFormContractNode(node)
}) })
} else if (common.isInheritanceSpecifier(node)) { } else if (common.isInheritanceSpecifier(node)) {
var currentContract = getCurrentContract(this) var currentContract = getCurrentContract(that)
var inheritsFromName = common.getInheritsFromName(node) var inheritsFromName = common.getInheritsFromName(node)
currentContract.inheritsFrom.push(inheritsFromName) currentContract.inheritsFrom.push(inheritsFromName)
// add variables from inherited contracts // 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) currentContract.stateVariables = currentContract.stateVariables.concat(inheritsFrom.stateVariables)
} else if (common.isFunctionDefinition(node)) { } else if (common.isFunctionDefinition(node)) {
setCurrentFunction(this, { setCurrentFunction(that, {
node: node, node: node,
relevantNodes: [], relevantNodes: [],
modifierInvocations: [], modifierInvocations: [],
...@@ -36,17 +61,17 @@ abstractAstView.prototype.builder = function (relevantNodeFilter, contractsOut) ...@@ -36,17 +61,17 @@ abstractAstView.prototype.builder = function (relevantNodeFilter, contractsOut)
parameters: getLocalParameters(node) parameters: getLocalParameters(node)
}) })
} else if (common.isModifierDefinition(node)) { } else if (common.isModifierDefinition(node)) {
setCurrentModifier(this, { setCurrentModifier(that, {
node: node, node: node,
relevantNodes: [], relevantNodes: [],
localVariables: getLocalVariables(node), localVariables: getLocalVariables(node),
parameters: getLocalParameters(node) parameters: getLocalParameters(node)
}) })
} else if (common.isModifierInvocation(node)) { } else if (common.isModifierInvocation(node)) {
if (!this.isFunctionNotModifier) throw new Error('abstractAstView.js: Found modifier invocation outside of function scope.') if (!that.isFunctionNotModifier) throw new Error('abstractAstView.js: Found modifier invocation outside of function scope.')
getCurrentFunction(this).modifierInvocations.push(node) getCurrentFunction(that).modifierInvocations.push(node)
} else if (relevantNodeFilter(node)) { } else if (relevantNodeFilter(node)) {
((this.isFunctionNotModifier) ? getCurrentFunction(this) : getCurrentModifier(this)).relevantNodes.push(node) ((that.isFunctionNotModifier) ? getCurrentFunction(that) : getCurrentModifier(that)).relevantNodes.push(node)
} }
} }
} }
......
...@@ -2,15 +2,16 @@ var name = 'Checks-Effects-Interaction pattern' ...@@ -2,15 +2,16 @@ var name = 'Checks-Effects-Interaction pattern'
var desc = 'Avoid potential reentrancy bugs' var desc = 'Avoid potential reentrancy bugs'
var categories = require('./categories') var categories = require('./categories')
var common = require('./staticAnalysisCommon') var common = require('./staticAnalysisCommon')
var callGraph = require('./functionCallGraph') var fcallGraph = require('./functionCallGraph')
var AbstractAst = require('./abstractAstView') var AbstractAst = require('./abstractAstView')
function checksEffectsInteraction () { function checksEffectsInteraction () {
this.contracts = [] this.contracts = []
var that = this
checksEffectsInteraction.prototype.visit = new AbstractAst().builder( checksEffectsInteraction.prototype.visit = new AbstractAst().builder(
(node) => common.isInteraction(node) || common.isEffect(node) || common.isLocalCallGraphRelevantNode(node), (node) => common.isInteraction(node) || common.isEffect(node) || common.isLocalCallGraphRelevantNode(node),
this.contracts that.contracts
) )
} }
...@@ -18,16 +19,16 @@ checksEffectsInteraction.prototype.report = function (compilationResults) { ...@@ -18,16 +19,16 @@ checksEffectsInteraction.prototype.report = function (compilationResults) {
var warnings = [] var warnings = []
var hasModifiers = this.contracts.some((item) => item.modifiers.length > 0) 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) => { this.contracts.forEach((contract) => {
contract.functions.forEach((func) => { contract.functions.forEach((func) => {
func.changesState = checkIfChangesState(common.getFullQuallyfiedFuncDefinitionIdent(contract.node, func.node, func.parameters), func.changesState = checkIfChangesState(common.getFullQuallyfiedFuncDefinitionIdent(contract.node, func.node, func.parameters),
getContext(cg, contract, func)) getContext(callGraph, contract, func))
}) })
contract.functions.forEach((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 funcName = common.getFullQuallyfiedFuncDefinitionIdent(contract.node, func.node, func.parameters)
var comments = (hasModifiers) ? '<br/><i>Note:</i>Modifiers are currently not considered by the this static analysis.' : '' var comments = (hasModifiers) ? '<br/><i>Note:</i>Modifiers are currently not considered by the this static analysis.' : ''
warnings.push({ warnings.push({
...@@ -42,8 +43,8 @@ checksEffectsInteraction.prototype.report = function (compilationResults) { ...@@ -42,8 +43,8 @@ checksEffectsInteraction.prototype.report = function (compilationResults) {
return warnings return warnings
} }
function getContext (cg, currentContract, func) { function getContext (callGraph, currentContract, func) {
return { cg: cg, currentContract: currentContract, stateVariables: getStateVariables(currentContract, func) } return { callGraph: callGraph, currentContract: currentContract, stateVariables: getStateVariables(currentContract, func) }
} }
function getStateVariables (contract, func) { function getStateVariables (contract, func) {
...@@ -65,14 +66,14 @@ function isPotentialVulnerableFunction (func, context) { ...@@ -65,14 +66,14 @@ function isPotentialVulnerableFunction (func, context) {
function isLocalCallWithStateChange (node, context) { function isLocalCallWithStateChange (node, context) {
if (common.isLocalCallGraphRelevantNode(node)) { 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 !func || (func && func.node.changesState)
} }
return false return false
} }
function checkIfChangesState (startFuncName, context) { 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 = { module.exports = {
......
...@@ -2,15 +2,16 @@ var name = 'Constant functions' ...@@ -2,15 +2,16 @@ var name = 'Constant functions'
var desc = 'Check for potentially constant functions' var desc = 'Check for potentially constant functions'
var categories = require('./categories') var categories = require('./categories')
var common = require('./staticAnalysisCommon') var common = require('./staticAnalysisCommon')
var callGraph = require('./functionCallGraph') var fcallGraph = require('./functionCallGraph')
var AbstractAst = require('./abstractAstView') var AbstractAst = require('./abstractAstView')
function constantFunctions () { function constantFunctions () {
this.contracts = [] this.contracts = []
var that = this
constantFunctions.prototype.visit = new AbstractAst().builder( 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.isExternalDirectCall(node) || common.isEffect(node) || common.isLocalCallGraphRelevantNode(node) || common.isInlineAssembly(node),
this.contracts that.contracts
) )
} }
...@@ -18,17 +19,15 @@ constantFunctions.prototype.report = function (compilationResults) { ...@@ -18,17 +19,15 @@ constantFunctions.prototype.report = function (compilationResults) {
var warnings = [] var warnings = []
var hasModifiers = this.contracts.some((item) => item.modifiers.length > 0) 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) => { this.contracts.forEach((contract) => {
if (!common.isFullyImplementedContract(contract.node)) return
contract.functions.forEach((func) => { contract.functions.forEach((func) => {
func.potentiallyshouldBeConst = checkIfShouldBeConstant(common.getFullQuallyfiedFuncDefinitionIdent(contract.node, func.node, func.parameters), 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) { if (common.isConstantFunction(func.node) !== func.potentiallyshouldBeConst) {
var funcName = common.getFullQuallyfiedFuncDefinitionIdent(contract.node, func.node, func.parameters) var funcName = common.getFullQuallyfiedFuncDefinitionIdent(contract.node, func.node, func.parameters)
var comments = (hasModifiers) ? '<br/><i>Note:</i>Modifiers are currently not considered by the this static analysis.' : '' var comments = (hasModifiers) ? '<br/><i>Note:</i>Modifiers are currently not considered by the this static analysis.' : ''
...@@ -52,8 +51,8 @@ constantFunctions.prototype.report = function (compilationResults) { ...@@ -52,8 +51,8 @@ constantFunctions.prototype.report = function (compilationResults) {
return warnings return warnings
} }
function getContext (cg, currentContract, func) { function getContext (callGraph, currentContract, func) {
return { cg: cg, currentContract: currentContract, stateVariables: getStateVariables(currentContract, func) } return { callGraph: callGraph, currentContract: currentContract, stateVariables: getStateVariables(currentContract, func) }
} }
function getStateVariables (contract, func) { function getStateVariables (contract, func) {
...@@ -61,7 +60,7 @@ function getStateVariables (contract, func) { ...@@ -61,7 +60,7 @@ function getStateVariables (contract, func) {
} }
function checkIfShouldBeConstant (startFuncName, context) { function checkIfShouldBeConstant (startFuncName, context) {
return !callGraph.analyseCallGraph(context.cg, startFuncName, context, isConstBreaker) return !fcallGraph.analyseCallGraph(context.callGraph, startFuncName, context, isConstBreaker)
} }
function isConstBreaker (node, context) { function isConstBreaker (node, context) {
...@@ -74,7 +73,7 @@ function isConstBreaker (node, context) { ...@@ -74,7 +73,7 @@ function isConstBreaker (node, context) {
function isCallOnNonConstExternalInterfaceFunction (node, context) { function isCallOnNonConstExternalInterfaceFunction (node, context) {
if (common.isExternalDirectCall(node)) { 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 !func || (func && !common.isConstantFunction(func.node.node))
} }
return false return false
......
...@@ -16,6 +16,30 @@ function buildLocalFuncCallGraphInternal (functions, nodeFilter, extractNodeIden ...@@ -16,6 +16,30 @@ function buildLocalFuncCallGraphInternal (functions, nodeFilter, extractNodeIden
return callGraph 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) { function buildGlobalFuncCallGraph (contracts) {
var callGraph = {} var callGraph = {}
contracts.forEach((contract) => { contracts.forEach((contract) => {
...@@ -29,31 +53,39 @@ function buildGlobalFuncCallGraph (contracts) { ...@@ -29,31 +53,39 @@ function buildGlobalFuncCallGraph (contracts) {
return callGraph 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) { function analyseCallGraphInternal (callGraph, funcName, context, combinator, nodeCheck, visited) {
var current = resolveCallGraphSymbol(cg, funcName) var current = resolveCallGraphSymbol(callGraph, funcName)
if (current === undefined || visited[funcName] === true) return true if (current === undefined || visited[funcName] === true) return true
visited[funcName] = true visited[funcName] = true
return combinator(current.node.relevantNodes.reduce((acc, val) => combinator(acc, nodeCheck(val, context)), false), 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) { function resolveCallGraphSymbol (callGraph, funcName) {
return resolveCallGraphSymbolInternal(cg, funcName, false) return resolveCallGraphSymbolInternal(callGraph, funcName, false)
} }
function resolveCallGraphSymbolInternal (cg, funcName, silent) { function resolveCallGraphSymbolInternal (callGraph, funcName, silent) {
var current var current
if (funcName.includes('.')) { if (funcName.includes('.')) {
var parts = funcName.split('.') var parts = funcName.split('.')
var contractPart = parts[0] var contractPart = parts[0]
var functionPart = parts[1] var functionPart = parts[1]
var currentContract = cg[contractPart] var currentContract = callGraph[contractPart]
if (!(currentContract === undefined)) { if (!(currentContract === undefined)) {
current = currentContract.functions[funcName] current = currentContract.functions[funcName]
// resolve inheritance hierarchy // resolve inheritance hierarchy
...@@ -61,7 +93,7 @@ function resolveCallGraphSymbolInternal (cg, funcName, silent) { ...@@ -61,7 +93,7 @@ function resolveCallGraphSymbolInternal (cg, funcName, silent) {
// resolve inheritance lookup in linearized fashion // resolve inheritance lookup in linearized fashion
var inheritsFromNames = currentContract.contract.inheritsFrom.reverse() var inheritsFromNames = currentContract.contract.inheritsFrom.reverse()
for (var i = 0; i < inheritsFromNames.length; i++) { 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 if (!(res === undefined)) return res
} }
} }
......
...@@ -3,8 +3,8 @@ module.exports = [ ...@@ -3,8 +3,8 @@ module.exports = [
require('./gasCosts'), require('./gasCosts'),
require('./thisLocal'), require('./thisLocal'),
require('./checksEffectsInteraction'), require('./checksEffectsInteraction'),
require('./constantFunctions'), require('./constantFunctions')
require('./inlineAssembly') // require('./inlineAssembly'),
// require('./blockTimestamp'), // require('./blockTimestamp'),
// require('./lowLevelCalls'), // require('./lowLevelCalls'),
// require('./blockBlockhash') // require('./blockBlockhash')
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment