Skip to content

Commit

Permalink
Static Analysis: library support
Browse files Browse the repository at this point in the history
  • Loading branch information
@chriseth
Michael Fröwis authored and chriseth committed May 17, 2017
1 parent f2068ea commit dc7dc11
Show file tree
Hide file tree
Showing 6 changed files with 200 additions and 29 deletions.
6 changes: 3 additions & 3 deletions src/app/staticanalysis/modules/checksEffectsInteraction.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Expand All @@ -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) ? '<br/><i>Note:</i>Modifiers are currently not considered by the this static analysis.' : ''
warnings.push({
warning: `Potential Violation of Checks-Effects-Interaction pattern in <i>${funcName}</i>: Could potentially lead to re-entrancy vulnerability.`,
warning: `Potential Violation of Checks-Effects-Interaction pattern in <i>${funcName}</i>: Could potentially lead to re-entrancy vulnerability.${comments}`,
location: func.src,
more: 'http://solidity.readthedocs.io/en/develop/security-considerations.html#re-entrancy'
})
Expand Down
8 changes: 4 additions & 4 deletions src/app/staticanalysis/modules/constantFunctions.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand All @@ -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) ? '<br/><i>Note:</i>Modifiers are currently not considered by the this static analysis.' : ''
if (func.potentiallyshouldBeConst) {
warnings.push({
warning: `<i>${funcName}</i>: Potentially should be constant but is not.`,
warning: `<i>${funcName}</i>: 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: `<i>${funcName}</i>: Is constant but potentially should not be.`,
warning: `<i>${funcName}</i>: Is constant but potentially should not be.${comments}`,
location: func.src,
more: 'http://solidity.readthedocs.io/en/develop/contracts.html#constant-functions'
})
Expand Down
34 changes: 29 additions & 5 deletions src/app/staticanalysis/modules/staticAnalysisCommon.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,8 @@ var basicRegex = {
EXTERNALFUNCTIONTYPE: '^function \\(.*\\).* external',
CONSTANTFUNCTIONTYPE: '^function \\(.*\\).* constant',
REFTYPE: '( storage )|(mapping\\()|(\\[\\])',
FUNCTIONSIGNATURE: '^function \\(([^\\(]*)\\)'
FUNCTIONSIGNATURE: '^function \\(([^\\(]*)\\)',
LIBRARYTYPE: '^type\\(library (.*)\\)'
}

var basicFunctionTypes = {
Expand Down Expand Up @@ -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
}

Expand Down Expand Up @@ -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')
}

Expand Down Expand Up @@ -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) {
Expand All @@ -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) {
Expand All @@ -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)
}
Expand Down Expand Up @@ -403,6 +423,8 @@ module.exports = {
getExternalDirectCallMemberName: getExternalDirectCallMemberName,
getFunctionDefinitionName: getFunctionDefinitionName,
getFunctionCallTypeParameterType: getFunctionCallTypeParameterType,
getLibraryCallContractName: getLibraryCallContractName,
getLibraryCallMemberName: getLibraryCallMemberName,
getFullQualifiedFunctionCallIdent: getFullQualifiedFunctionCallIdent,
getFullQuallyfiedFuncDefinitionIdent: getFullQuallyfiedFuncDefinitionIdent,
getStateVariableDeclarationsFormContractNode: getStateVariableDeclarationsFormContractNode,
Expand All @@ -416,6 +438,7 @@ module.exports = {
isBlockBlockHashAccess: isBlockBlockHashAccess,
isThisLocalCall: isThisLocalCall,
isSuperLocalCall: isSuperLocalCall,
isLibraryCall: isLibraryCall,
isLocalCallGraphRelevantNode: isLocalCallGraphRelevantNode,
isLocalCall: isLocalCall,
isWriteOnStateVariable: isWriteOnStateVariable,
Expand All @@ -427,6 +450,7 @@ module.exports = {
isLowLevelSendInst: isLLSend,
isExternalDirectCall: isExternalDirectCall,
isFullyImplementedContract: isFullyImplementedContract,
isLibrary: isLibrary,
isCallToNonConstLocalFunction: isCallToNonConstLocalFunction,
isPlusPlusUnaryOperation: isPlusPlusUnaryOperation,
isMinusMinusUnaryOperation: isMinusMinusUnaryOperation,
Expand Down
96 changes: 91 additions & 5 deletions test/staticanalysis/staticAnalysisCommon-test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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()',
Expand Down Expand Up @@ -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')
})

Expand Down Expand Up @@ -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' } }
Expand Down Expand Up @@ -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')
Expand Down Expand Up @@ -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 = {
Expand Down
31 changes: 19 additions & 12 deletions test/staticanalysis/staticAnalysisIntegration-test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,8 @@ var testFiles = [
'notReentrant.sol',
'structReentrant.sol',
'thisLocal.sol',
'globals.sol'
'globals.sol',
'library.sol'
]

var testFileAsts = {}
Expand Down Expand Up @@ -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) => {
Expand All @@ -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) => {
Expand All @@ -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) => {
Expand All @@ -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) => {
Expand All @@ -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) => {
Expand All @@ -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) => {
Expand Down
Loading

0 comments on commit dc7dc11

Please sign in to comment.