Skip to content

Commit

Permalink
Static Analysis: ChecksEffectsInteraction, Constant Function, Inline …
Browse files Browse the repository at this point in the history
…Assembly. Without integration tests and modifier support
  • Loading branch information
Michael Fröwis authored and chriseth committed May 17, 2017
1 parent 01d700d commit 11917ff
Show file tree
Hide file tree
Showing 26 changed files with 1,288 additions and 25 deletions.
93 changes: 93 additions & 0 deletions src/app/staticanalysis/modules/abstractAstView.js
Original file line number Diff line number Diff line change
@@ -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
3 changes: 2 additions & 1 deletion src/app/staticanalysis/modules/categories.js
Original file line number Diff line number Diff line change
@@ -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'}
}
81 changes: 81 additions & 0 deletions src/app/staticanalysis/modules/checksEffectsInteraction.js
Original file line number Diff line number Diff line change
@@ -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 <i>${funcName}</i>: 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
}
86 changes: 86 additions & 0 deletions src/app/staticanalysis/modules/constantFunctions.js
Original file line number Diff line number Diff line change
@@ -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: `<i>${funcName}</i>: 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: `<i>${funcName}</i>: 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
}
84 changes: 84 additions & 0 deletions src/app/staticanalysis/modules/functionCallGraph.js
Original file line number Diff line number Diff line change
@@ -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
}
29 changes: 29 additions & 0 deletions src/app/staticanalysis/modules/inlineAssembly.js
Original file line number Diff line number Diff line change
@@ -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
}
5 changes: 4 additions & 1 deletion src/app/staticanalysis/modules/list.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,8 @@
module.exports = [
require('./txOrigin'),
require('./gasCosts'),
require('./thisLocal')
require('./thisLocal'),
require('./checksEffectsInteraction'),
require('./constantFunctions'),
require('./inlineAssembly')
]
Loading

0 comments on commit 11917ff

Please sign in to comment.