Merge pull request #909 from soad003/selfdestructTerminal

Selfdestruct terminal

remix-analyzer/src/solidity-analyzer/modules/algorithmCategories.js

@@ -0,0 +1,9 @@
+/**
+  * Should be used to categorize different modules, main reason is to give users feedback if the modules
+  * Produce exact results or have false positives and negatives in them
+  * A further category could be approximate if some form of approximation is used
+*/
+module.exports = {
+  EXACT: { hasFalsePositives: false, hasFalseNegatives: false, id: 'EXACT' },
+  HEURISTIC: { hasFalsePositives: true, hasFalseNegatives: true, id: 'HEURI' }
+}

remix-analyzer/src/solidity-analyzer/modules/assignAndCompare.js

@@ -2,6 +2,7 @@ var name = 'Result not used: '
 var desc = 'The result of an operation was not used.'
 var categories = require('./categories')
 var common = require('./staticAnalysisCommon')
+var algo = require('./algorithmCategories')
 
 function assignAndCompare () {
   this.warningNodes = []
@@ -24,5 +25,6 @@ module.exports = {
   name: name,
   description: desc,
   category: categories.MISC,
+  algorithm: algo.EXACT,
   Module: assignAndCompare
 }

remix-analyzer/src/solidity-analyzer/modules/blockBlockhash.js

@@ -2,6 +2,7 @@ var name = 'Block.blockhash usage: '
 var desc = 'Semantics maybe unclear'
 var categories = require('./categories')
 var common = require('./staticAnalysisCommon')
+var algo = require('./algorithmCategories')
 
 function blockBlockhash () {
   this.warningNodes = []
@@ -27,6 +28,7 @@ module.exports = {
   name: name,
   description: desc,
   category: categories.SECURITY,
+  algorithm: algo.EXACT,
   Module: blockBlockhash
 }
 

remix-analyzer/src/solidity-analyzer/modules/blockTimestamp.js

@@ -2,6 +2,7 @@ var name = 'Block timestamp: '
 var desc = 'Semantics maybe unclear'
 var categories = require('./categories')
 var common = require('./staticAnalysisCommon')
+var algo = require('./algorithmCategories')
 
 function blockTimestamp () {
   this.warningNowNodes = []
@@ -35,6 +36,7 @@ module.exports = {
   name: name,
   description: desc,
   category: categories.SECURITY,
+  algorithm: algo.EXACT,
   Module: blockTimestamp
 }
 

remix-analyzer/src/solidity-analyzer/modules/checksEffectsInteraction.js

@@ -4,6 +4,7 @@ var categories = require('./categories')
 var common = require('./staticAnalysisCommon')
 var fcallGraph = require('./functionCallGraph')
 var AbstractAst = require('./abstractAstView')
+var algo = require('./algorithmCategories')
 
 function checksEffectsInteraction () {
   this.abstractAst = new AbstractAst()
@@ -84,5 +85,6 @@ module.exports = {
   name: name,
   description: desc,
   category: categories.SECURITY,
+  algorithm: algo.HEURISTIC,
   Module: checksEffectsInteraction
 }

remix-analyzer/src/solidity-analyzer/modules/constantFunctions.js

@@ -4,6 +4,7 @@ var categories = require('./categories')
 var common = require('./staticAnalysisCommon')
 var fcallGraph = require('./functionCallGraph')
 var AbstractAst = require('./abstractAstView')
+var algo = require('./algorithmCategories')
 
 function constantFunctions () {
   this.abstractAst = new AbstractAst()
@@ -104,5 +105,6 @@ module.exports = {
   name: name,
   description: desc,
   category: categories.MISC,
+  algorithm: algo.HEURISTIC,
   Module: constantFunctions
 }

remix-analyzer/src/solidity-analyzer/modules/deleteDynamicArrays.js

@@ -2,6 +2,7 @@ var name = 'Delete on dynamic Array: '
 var desc = 'Use require and appropriately'
 var categories = require('./categories')
 var common = require('./staticAnalysisCommon')
+var algo = require('./algorithmCategories')
 
 function deleteDynamicArrays () {
   this.rel = []
@@ -25,5 +26,6 @@ module.exports = {
   name: name,
   description: desc,
   category: categories.GAS,
+  algorithm: algo.EXACT,
   Module: deleteDynamicArrays
 }

remix-analyzer/src/solidity-analyzer/modules/gasCosts.js

@@ -1,6 +1,7 @@
 var name = 'Gas costs: '
 var desc = 'Warn if the gas requirements of functions are too high.'
 var categories = require('./categories')
+var algo = require('./algorithmCategories')
 
 function gasCosts () {
 }
@@ -61,5 +62,6 @@ module.exports = {
   name: name,
   description: desc,
   category: categories.GAS,
+  algorithm: algo.EXACT,
   Module: gasCosts
 }

remix-analyzer/src/solidity-analyzer/modules/guardConditions.js

@@ -2,6 +2,7 @@ var name = 'Guard Conditions: '
 var desc = 'Use require and appropriately'
 var categories = require('./categories')
 var common = require('./staticAnalysisCommon')
+var algo = require('./algorithmCategories')
 
 function guardConditions () {
   this.guards = []
@@ -25,5 +26,6 @@ module.exports = {
   name: name,
   description: desc,
   category: categories.MISC,
+  algorithm: algo.EXACT,
   Module: guardConditions
 }

remix-analyzer/src/solidity-analyzer/modules/inlineAssembly.js

@@ -2,6 +2,7 @@ var name = 'Inline assembly: '
 var desc = 'Use of Inline Assembly'
 var categories = require('./categories')
 var common = require('./staticAnalysisCommon')
+var algo = require('./algorithmCategories')
 
 function inlineAssembly () {
   this.inlineAssNodes = []
@@ -26,5 +27,6 @@ module.exports = {
   name: name,
   description: desc,
   category: categories.SECURITY,
+  algorithm: algo.EXACT,
   Module: inlineAssembly
 }

remix-analyzer/src/solidity-analyzer/modules/intDivisionTruncate.js

@@ -2,6 +2,7 @@ var name = 'Data Trucated: '
 var desc = 'Division on int/uint values truncates the result.'
 var categories = require('./categories')
 var common = require('./staticAnalysisCommon')
+var algo = require('./algorithmCategories')
 
 function intDivitionTruncate () {
   this.warningNodes = []
@@ -14,7 +15,7 @@ intDivitionTruncate.prototype.visit = function (node) {
 intDivitionTruncate.prototype.report = function (compilationResults) {
   return this.warningNodes.map(function (item, i) {
     return {
-      warning: 'Division of integer values yields an integer value again. That means eg. a / 100 = 0 instead of 0.a since the result is an integer again. This does not hold for division of (only) literal values since those yield rational constants.',
+      warning: 'Division of integer values yields an integer value again. That means e.g. 10 / 100 = 0 instead of 0.1 since the result is an integer again. This does not hold for division of (only) literal values since those yield rational constants.',
       location: item.src
     }
   })
@@ -24,5 +25,6 @@ module.exports = {
   name: name,
   description: desc,
   category: categories.MISC,
+  algorithm: algo.EXACT,
   Module: intDivitionTruncate
 }

remix-analyzer/src/solidity-analyzer/modules/lowLevelCalls.js

@@ -2,6 +2,7 @@ var name = 'Low level calls: '
 var desc = 'Semantics maybe unclear'
 var categories = require('./categories')
 var common = require('./staticAnalysisCommon')
+var algo = require('./algorithmCategories')
 
 function lowLevelCalls () {
   this.llcNodes = []
@@ -59,6 +60,7 @@ module.exports = {
   name: name,
   description: desc,
   category: categories.SECURITY,
+  algorithm: algo.EXACT,
   Module: lowLevelCalls
 }
 

remix-analyzer/src/solidity-analyzer/modules/noReturn.js

@@ -3,6 +3,7 @@ var desc = 'Function with return type is not returning'
 var categories = require('./categories')
 var common = require('./staticAnalysisCommon')
 var AbstractAst = require('./abstractAstView')
+var algo = require('./algorithmCategories')
 
 function noReturn () {
   this.abstractAst = new AbstractAst()
@@ -69,5 +70,6 @@ module.exports = {
   name: name,
   description: desc,
   category: categories.MISC,
+  algorithm: algo.EXACT,
   Module: noReturn
 }

remix-analyzer/src/solidity-analyzer/modules/selfdestruct.js

@@ -2,30 +2,58 @@ var name = 'Selfdestruct: '
 var desc = 'Be aware of caller contracts.'
 var categories = require('./categories')
 var common = require('./staticAnalysisCommon')
+var AbstractAst = require('./abstractAstView')
+var algo = require('./algorithmCategories')
 
 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('selfdestruct.js no visit function set upon construction') }
+
+selfdestruct.prototype.report = function () { throw new Error('selfdestruct.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 = {
   name: name,
   description: desc,
   category: categories.SECURITY,
+  algorithm: algo.HEURISTIC,
   Module: selfdestruct
 }

remix-analyzer/src/solidity-analyzer/modules/similarVariableNames.js

@@ -6,6 +6,7 @@ var AbstractAst = require('./abstractAstView')
 var levenshtein = require('fast-levenshtein')
 var remixLib = require('remix-lib')
 var util = remixLib.util
+var algo = require('./algorithmCategories')
 
 function similarVariableNames () {
   this.abstractAst = new AbstractAst()
@@ -82,5 +83,6 @@ module.exports = {
   name: name,
   description: desc,
   category: categories.MISC,
+  algorithm: algo.EXACT,
   Module: similarVariableNames
 }

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) || isReturn(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,

remix-analyzer/src/solidity-analyzer/modules/thisLocal.js

@@ -2,6 +2,7 @@ var name = 'This on local calls: '
 var desc = 'Invocation of local functions via this'
 var categories = require('./categories')
 var common = require('./staticAnalysisCommon')
+var algo = require('./algorithmCategories')
 
 function thisLocal () {
   this.warningNodes = []
@@ -25,5 +26,6 @@ module.exports = {
   name: name,
   description: desc,
   category: categories.GAS,
+  algorithm: algo.EXACT,
   Module: thisLocal
 }

remix-analyzer/src/solidity-analyzer/modules/txOrigin.js

@@ -1,6 +1,7 @@
 var name = 'Transaction origin: '
 var desc = 'Warn if tx.origin is used'
 var categories = require('./categories')
+var algo = require('./algorithmCategories')
 
 function txOrigin () {
   this.txOriginNodes = []
@@ -31,5 +32,6 @@ module.exports = {
   name: name,
   description: desc,
   category: categories.SECURITY,
+  algorithm: algo.EXACT,
   Module: txOrigin
 }

remix-analyzer/test/analysis/staticAnalysisIntegration-test.js

@@ -466,15 +466,15 @@ 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
+    'intDivisionTruncate.sol': 5
   }
 
   runModuleOnFiles(module, t, (file, report) => {

remix-analyzer/test/analysis/test-contracts/intDivisionTruncate.sol

@@ -21,8 +21,18 @@ contract CharityCampaign {
         return fee;
     }
 
-    function endCampaign() public {
+    function endCampaign() public returns (bool) {
         require(msg.sender == processor || msg.sender == beneficiary);
         selfdestruct(beneficiary);
+        return true;
+    }
+
+    // FALSE POSITIVE FOR SELFDESTRUCT TERMINAL 
+    function endAmbiguous() public {
+        if(msg.sender == 0x0) {
+            selfdestruct(beneficiary);
+        } else {
+            selfdestruct(processor);
+        }
     }
 }

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;
   }
 }