diff --git a/remix-analyzer/src/solidity-analyzer/modules/selfdestruct.js b/remix-analyzer/src/solidity-analyzer/modules/selfdestruct.js index 14b24f9e3c..e9ac8557a1 100644 --- a/remix-analyzer/src/solidity-analyzer/modules/selfdestruct.js +++ b/remix-analyzer/src/solidity-analyzer/modules/selfdestruct.js @@ -2,25 +2,51 @@ var name = 'Selfdestruct: ' var desc = 'Be aware of caller contracts.' var categories = require('./categories') var common = require('./staticAnalysisCommon') +var AbstractAst = require('./abstractAstView') function selfdestruct () { - this.relevantNodes = [] -} + this.abstractAst = new AbstractAst() + + this.visit = this.abstractAst.build_visit( + (node) => common.isStatement(node) || + common.isSelfdestructCall(node) + ) -selfdestruct.prototype.visit = function (node) { - if (common.isSelfdestructCall(node)) { - this.relevantNodes.push(node) - } + this.report = this.abstractAst.build_report(report) } -selfdestruct.prototype.report = function () { - return this.relevantNodes.map(function (item, i) { - return { - warning: 'Use of selfdestruct: can block calling contracts unexpectedly. Be especially careful if this contract is planned to be used by other contracts (i.e. library contracts, interactions). Selfdestruction of the callee contract can leave callers in an inoperable state.', - location: item.src, - more: 'https://paritytech.io/blog/security-alert.html' - } +selfdestruct.prototype.visit = function () { throw new Error('constantFunctions.js no visit function set upon construction') } + +selfdestruct.prototype.report = function () { throw new Error('constantFunctions.js no report function set upon construction') } + +function report (contracts, multipleContractsWithSameName) { + var warnings = [] + + contracts.forEach((contract) => { + contract.functions.forEach((func) => { + let hasSelf = false + func.relevantNodes.forEach((node) => { + if (common.isSelfdestructCall(node)) { + warnings.push({ + warning: 'Use of selfdestruct: can block calling contracts unexpectedly. Be especially careful if this contract is planned to be used by other contracts (i.e. library contracts, interactions). Selfdestruction of the callee contract can leave callers in an inoperable state.', + location: node.src, + more: 'https://paritytech.io/blog/security-alert.html' + }) + hasSelf = true + } + if (common.isStatement(node) && hasSelf) { + warnings.push({ + warning: 'Use of selfdestruct: No code after selfdestruct is executed. Selfdestruct is a terminal.', + location: node.src, + more: 'http://solidity.readthedocs.io/en/develop/introduction-to-smart-contracts.html#self-destruct' + }) + hasSelf = false + } + }) + }) }) + + return warnings } module.exports = { diff --git a/remix-analyzer/src/solidity-analyzer/modules/staticAnalysisCommon.js b/remix-analyzer/src/solidity-analyzer/modules/staticAnalysisCommon.js index 51929708d0..19f7984e3b 100644 --- a/remix-analyzer/src/solidity-analyzer/modules/staticAnalysisCommon.js +++ b/remix-analyzer/src/solidity-analyzer/modules/staticAnalysisCommon.js @@ -396,6 +396,14 @@ function isFunctionDefinition (node) { return nodeType(node, exactMatch(nodeTypes.FUNCTIONDEFINITION)) } +function isStatement (node) { + return nodeType(node, 'Statement$') || isBlock(node) +} + +function isBlock (node) { + return nodeType(node, exactMatch(nodeTypes.BLOCK)) +} + function isModifierDefinition (node) { return nodeType(node, exactMatch(nodeTypes.MODIFIERDEFINITION)) } @@ -1009,6 +1017,8 @@ module.exports = { isInlineAssembly: isInlineAssembly, isNewExpression: isNewExpression, isReturn: isReturn, + isStatement: isStatement, + isBlock: isBlock, // #################### Constants nodeTypes: nodeTypes, diff --git a/remix-analyzer/test/analysis/staticAnalysisIntegration-test.js b/remix-analyzer/test/analysis/staticAnalysisIntegration-test.js index 0cf349100c..b8b7cc031b 100644 --- a/remix-analyzer/test/analysis/staticAnalysisIntegration-test.js +++ b/remix-analyzer/test/analysis/staticAnalysisIntegration-test.js @@ -466,12 +466,12 @@ test('Integration test selfdestruct.js', function (t) { 'notReentrant.sol': 0, 'structReentrant.sol': 0, 'thisLocal.sol': 0, - 'globals.sol': 1, + 'globals.sol': 2, 'library.sol': 0, 'transfer.sol': 0, 'ctor.sol': 0, 'forgottenReturn.sol': 0, - 'selfdestruct.sol': 2, + 'selfdestruct.sol': 3, 'deleteDynamicArray.sol': 0, 'blockLevelCompare.sol': 0, 'intDivisionTruncate.sol': 1 diff --git a/remix-analyzer/test/analysis/test-contracts/selfdestruct.sol b/remix-analyzer/test/analysis/test-contracts/selfdestruct.sol index d2e31dfdb0..2f56dfc2e4 100644 --- a/remix-analyzer/test/analysis/test-contracts/selfdestruct.sol +++ b/remix-analyzer/test/analysis/test-contracts/selfdestruct.sol @@ -1,5 +1,6 @@ contract sd { + uint x = 0; function() public payable { } function c () public constant { @@ -8,5 +9,6 @@ contract sd { function b () public payable { selfdestruct(address(0xdeadbeef)); + x = 1; } } \ No newline at end of file