Static Analysis: Warn on selfdestruct, functions with selfdestruct cannot be const

pull/3094/head
soad003 7 years ago
parent 59ec63778b
commit b736eb3e55
  1. 18
      src/app/staticanalysis/modules/constantFunctions.js
  2. 3
      src/app/staticanalysis/modules/list.js
  3. 33
      src/app/staticanalysis/modules/selfdestruct.js
  4. 17
      src/app/staticanalysis/modules/staticAnalysisCommon.js
  5. 70
      test/staticanalysis/staticAnalysisIntegration-test.js
  6. 13
      test/staticanalysis/test-contracts/selfdestruct.sol

@ -10,7 +10,14 @@ function constantFunctions () {
this.abstractAst = new AbstractAst()
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)
(node) => common.isLowLevelCall(node) ||
common.isTransfer(node) ||
common.isExternalDirectCall(node) ||
common.isEffect(node) ||
common.isLocalCallGraphRelevantNode(node) ||
common.isInlineAssembly(node) ||
common.isNewExpression(node) ||
common.isSelfdestructCall(node)
)
this.report = this.abstractAst.build_report(report)
@ -28,8 +35,12 @@ function report (contracts, multipleContractsWithSameName) {
contracts.forEach((contract) => {
contract.functions.forEach((func) => {
func.potentiallyshouldBeConst = checkIfShouldBeConstant(common.getFullQuallyfiedFuncDefinitionIdent(contract.node, func.node, func.parameters),
if (common.isPayableFunction(func.node)) {
func.potentiallyshouldBeConst = false
} else {
func.potentiallyshouldBeConst = checkIfShouldBeConstant(common.getFullQuallyfiedFuncDefinitionIdent(contract.node, func.node, func.parameters),
getContext(callGraph, contract, func))
}
})
contract.functions.filter((func) => common.hasFunctionBody(func.node)).forEach((func) => {
@ -76,7 +87,8 @@ function isConstBreaker (node, context) {
isCallOnNonConstExternalInterfaceFunction(node, context) ||
common.isCallToNonConstLocalFunction(node) ||
common.isInlineAssembly(node) ||
common.isNewExpression(node)
common.isNewExpression(node) ||
common.isSelfdestructCall(node)
}
function isCallOnNonConstExternalInterfaceFunction (node, context) {

@ -9,5 +9,6 @@ module.exports = [
require('./blockTimestamp'),
require('./lowLevelCalls'),
require('./blockBlockhash'),
require('./noReturn')
require('./noReturn'),
require('./selfdestruct')
]

@ -0,0 +1,33 @@
var name = 'Selfdestruct: '
var desc = 'Be aware of caller contracts.'
var categories = require('./categories')
var common = require('./staticAnalysisCommon')
var yo = require('yo-yo')
function selfdestruct () {
this.relevantNodes = []
}
selfdestruct.prototype.visit = function (node) {
if (common.isSelfdestructCall(node)) {
this.relevantNodes.push(node)
}
}
selfdestruct.prototype.report = function () {
return this.relevantNodes.map(function (item, i) {
return {
warning: yo`<span>Use of selfdestruct: can block calling contracts unexpectedly<br />
Please, be especially carefull if this contract is referenced by other contracts (i.e. library contracts, interactions). Selfdestruction of called contracts can render callers inoperable.</span>`,
location: item.src,
more: 'https://www.coindesk.com/ethereum-client-bug-freezes-user-funds-fallout-remains-uncertain/'
}
})
}
module.exports = {
name: name,
description: desc,
category: categories.SECURITY,
Module: selfdestruct
}

@ -406,6 +406,10 @@ function isBuiltinFunctionCall (node) {
return isLocalCall(node) && builtinFunctions[getLocalCallName(node) + '(' + getFunctionCallTypeParameterType(node) + ')'] === true
}
function isSelfdestructCall (node) {
return isBuiltinFunctionCall(node) && getLocalCallName(node) === 'selfdestruct'
}
/**
* True if is storage variable declaration
* @node {ASTNode} some AstNode
@ -466,6 +470,17 @@ function isConstantFunction (node) {
)
}
/**
* True if is function defintion has payable modifier
* @node {ASTNode} some AstNode
* @return {bool}
*/
function isPayableFunction (node) {
return isFunctionDefinition(node) && (
node.attributes.stateMutability === 'payable'
)
}
/**
* True if unary increment operation
* @node {ASTNode} some AstNode
@ -788,6 +803,7 @@ module.exports = {
isPlusPlusUnaryOperation: isPlusPlusUnaryOperation,
isMinusMinusUnaryOperation: isMinusMinusUnaryOperation,
isBuiltinFunctionCall: isBuiltinFunctionCall,
isSelfdestructCall: isSelfdestructCall,
// #################### Trivial Node Identification
isFunctionDefinition: isFunctionDefinition,
@ -799,6 +815,7 @@ module.exports = {
isAssignment: isAssignment,
isContractDefinition: isContractDefinition,
isConstantFunction: isConstantFunction,
isPayableFunction: isPayableFunction,
isInlineAssembly: isInlineAssembly,
isNewExpression: isNewExpression,
isReturn: isReturn,

@ -28,7 +28,8 @@ var testFiles = [
'library.sol',
'transfer.sol',
'ctor.sol',
'forgottenReturn.sol'
'forgottenReturn.sol',
'selfdestruct.sol'
]
var testFileAsts = {}
@ -60,7 +61,8 @@ test('Integration test thisLocal.js', function (t) {
'library.sol': 0,
'transfer.sol': 0,
'ctor.sol': 0,
'forgottenReturn.sol': 0
'forgottenReturn.sol': 0,
'selfdestruct.sol': 0
}
runModuleOnFiles(module, t, (file, report) => {
@ -90,7 +92,8 @@ test('Integration test checksEffectsInteraction.js', function (t) {
'library.sol': 1,
'transfer.sol': 1,
'ctor.sol': 0,
'forgottenReturn.sol': 0
'forgottenReturn.sol': 0,
'selfdestruct.sol': 0
}
runModuleOnFiles(module, t, (file, report) => {
@ -120,7 +123,8 @@ test('Integration test constantFunctions.js', function (t) {
'library.sol': 1,
'transfer.sol': 0,
'ctor.sol': 0,
'forgottenReturn.sol': 0
'forgottenReturn.sol': 0,
'selfdestruct.sol': 1
}
runModuleOnFiles(module, t, (file, report) => {
@ -150,7 +154,8 @@ test('Integration test inlineAssembly.js', function (t) {
'library.sol': 0,
'transfer.sol': 0,
'ctor.sol': 0,
'forgottenReturn.sol': 0
'forgottenReturn.sol': 0,
'selfdestruct.sol': 0
}
runModuleOnFiles(module, t, (file, report) => {
@ -180,7 +185,8 @@ test('Integration test txOrigin.js', function (t) {
'library.sol': 0,
'transfer.sol': 0,
'ctor.sol': 0,
'forgottenReturn.sol': 0
'forgottenReturn.sol': 0,
'selfdestruct.sol': 0
}
runModuleOnFiles(module, t, (file, report) => {
@ -210,7 +216,8 @@ test('Integration test gasCosts.js', function (t) {
'library.sol': 1,
'transfer.sol': 1,
'ctor.sol': 0,
'forgottenReturn.sol': 3
'forgottenReturn.sol': 3,
'selfdestruct.sol': 0
}
runModuleOnFiles(module, t, (file, report) => {
@ -240,7 +247,8 @@ test('Integration test similarVariableNames.js', function (t) {
'library.sol': 0,
'transfer.sol': 0,
'ctor.sol': 1,
'forgottenReturn.sol': 0
'forgottenReturn.sol': 0,
'selfdestruct.sol': 0
}
runModuleOnFiles(module, t, (file, report) => {
@ -270,7 +278,8 @@ test('Integration test inlineAssembly.js', function (t) {
'library.sol': 0,
'transfer.sol': 0,
'ctor.sol': 0,
'forgottenReturn.sol': 0
'forgottenReturn.sol': 0,
'selfdestruct.sol': 0
}
runModuleOnFiles(module, t, (file, report) => {
@ -300,7 +309,8 @@ test('Integration test blockTimestamp.js', function (t) {
'library.sol': 0,
'transfer.sol': 0,
'ctor.sol': 0,
'forgottenReturn.sol': 0
'forgottenReturn.sol': 0,
'selfdestruct.sol': 0
}
runModuleOnFiles(module, t, (file, report) => {
@ -330,7 +340,8 @@ test('Integration test lowLevelCalls.js', function (t) {
'library.sol': 1,
'transfer.sol': 0,
'ctor.sol': 0,
'forgottenReturn.sol': 0
'forgottenReturn.sol': 0,
'selfdestruct.sol': 0
}
runModuleOnFiles(module, t, (file, report) => {
@ -360,7 +371,8 @@ test('Integration test blockBlockhash.js', function (t) {
'library.sol': 0,
'transfer.sol': 0,
'ctor.sol': 0,
'forgottenReturn.sol': 0
'forgottenReturn.sol': 0,
'selfdestruct.sol': 0
}
runModuleOnFiles(module, t, (file, report) => {
@ -390,7 +402,8 @@ test('Integration test noReturn.js', function (t) {
'library.sol': 0,
'transfer.sol': 0,
'ctor.sol': 0,
'forgottenReturn.sol': 1
'forgottenReturn.sol': 1,
'selfdestruct.sol': 0
}
runModuleOnFiles(module, t, (file, report) => {
@ -398,6 +411,37 @@ test('Integration test noReturn.js', function (t) {
})
})
test('Integration test selfdestruct.js', function (t) {
t.plan(testFiles.length)
var module = require('../../src/app/staticanalysis/modules/selfdestruct')
var lengthCheck = {
'KingOfTheEtherThrone.sol': 0,
'assembly.sol': 0,
'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,
'globals.sol': 1,
'library.sol': 0,
'transfer.sol': 0,
'ctor.sol': 0,
'forgottenReturn.sol': 0,
'selfdestruct.sol': 2
}
runModuleOnFiles(module, t, (file, report) => {
t.equal(report.length, lengthCheck[file], `${file} has right amount of selfdestruct warnings`)
})
})
// #################### Helpers
function runModuleOnFiles (module, t, cb) {
var statRunner = new StatRunner()

@ -0,0 +1,13 @@
contract sd {
uint120 x;
function() public payable { }
function c () public constant {
//x++;
selfdestruct(address(0xdeadbeef));
}
function b () public payable {
selfdestruct(address(0xdeadbeef));
}
}
Loading…
Cancel
Save