Static Analysis: checks effects interaction bugfix transfer, suggestions by chriseth

src/app/staticanalysis/modules/lowLevelCalls.js

@@ -38,7 +38,7 @@ lowLevelCalls.prototype.report = function (compilationResults) {
         morehref = `http://solidity.readthedocs.io/en/develop/contracts.html#libraries`
         break
       case common.lowLevelCallTypes.SEND:
-        text = `use of "send": "send" does not throw an exception when not successful, make sure you deal with the failure case accordingly. Use "transfer" whenever failure of the ether transfer should rollback the whole transaction. Additionally if you "send" ether to a contract the fallback function is called, the callees fallback function is very limited due to the limited amount of gas provided by "send". No state changes are possible but the callee can log the event or revert the transfer. "send" is syntactic sugar for a "call" to the fallback function with 2300 gas and a specified ether value. <br />`
+        text = `use of "send": "send" does not throw an exception when not successful, make sure you deal with the failure case accordingly. Use "transfer" whenever failure of the ether transfer should rollback the whole transaction. Note: if you "send/transfer" ether to a contract the fallback function is called, the callees fallback function is very limited due to the limited amount of gas provided by "send/transfer". No state changes are possible but the callee can log the event or revert the transfer. "send/transfer" is syntactic sugar for a "call" to the fallback function with 2300 gas and a specified ether value. <br />`
         morehref = `http://solidity.readthedocs.io/en/develop/security-considerations.html#sending-and-receiving-ether`
         break
     }

src/app/staticanalysis/modules/similarVariableNames.js

@@ -56,7 +56,7 @@ function findSimilarVarNames (vars) {
 }
 
 function isCommonPrefixedVersion (varName1, varName2) {
-  return (varName1.startsWith('_') || varName2.startsWith('_')) && (varName1.slice(1) === varName2 || varName1 === varName2.slice(1))
+  return (varName1.startsWith('_') && varName1.slice(1) === varName2) || (varName2.startsWith('_') && varName2.slice(1) === varName1)
 }
 
 function getFunctionVariables (contract, func) {

src/app/staticanalysis/modules/staticAnalysisCommon.js

@@ -403,7 +403,7 @@ function isStorageVariableDeclaration (node) {
  * @return {bool}
  */
 function isInteraction (node) {
-  return isLLCall(node) || isLLSend(node) || isExternalDirectCall(node)
+  return isLLCall(node) || isLLSend(node) || isExternalDirectCall(node) || isTransfer(node)
 }
 
 /**

test/staticanalysis/staticAnalysisIntegration-test.js

@@ -23,7 +23,9 @@ var testFiles = [
   'structReentrant.sol',
   'thisLocal.sol',
   'globals.sol',
-  'library.sol'
+  'library.sol',
+  'transfer.sol',
+  'ctor.sol'
 ]
 
 var testFileAsts = {}
@@ -52,7 +54,9 @@ test('Integration test thisLocal.js', function (t) {
     'structReentrant.sol': 0,
     'thisLocal.sol': 1,
     'globals.sol': 0,
-    'library.sol': 0
+    'library.sol': 0,
+    'transfer.sol': 0,
+    'ctor.sol': 0
   }
 
   runModuleOnFiles(module, t, (file, report) => {
@@ -79,7 +83,9 @@ test('Integration test checksEffectsInteraction.js', function (t) {
     'structReentrant.sol': 1,
     'thisLocal.sol': 0,
     'globals.sol': 1,
-    'library.sol': 1
+    'library.sol': 1,
+    'transfer.sol': 1,
+    'ctor.sol': 0
   }
 
   runModuleOnFiles(module, t, (file, report) => {
@@ -106,7 +112,9 @@ test('Integration test constantFunctions.js', function (t) {
     'structReentrant.sol': 0,
     'thisLocal.sol': 1,
     'globals.sol': 0,
-    'library.sol': 1
+    'library.sol': 1,
+    'transfer.sol': 0,
+    'ctor.sol': 0
   }
 
   runModuleOnFiles(module, t, (file, report) => {
@@ -133,7 +141,9 @@ test('Integration test inlineAssembly.js', function (t) {
     'structReentrant.sol': 0,
     'thisLocal.sol': 0,
     'globals.sol': 0,
-    'library.sol': 0
+    'library.sol': 0,
+    'transfer.sol': 0,
+    'ctor.sol': 0
   }
 
   runModuleOnFiles(module, t, (file, report) => {
@@ -160,7 +170,9 @@ test('Integration test txOrigin.js', function (t) {
     'structReentrant.sol': 0,
     'thisLocal.sol': 0,
     'globals.sol': 1,
-    'library.sol': 0
+    'library.sol': 0,
+    'transfer.sol': 0,
+    'ctor.sol': 0
   }
 
   runModuleOnFiles(module, t, (file, report) => {
@@ -187,7 +199,9 @@ test('Integration test gasCosts.js', function (t) {
     'structReentrant.sol': 1,
     'thisLocal.sol': 2,
     'globals.sol': 1,
-    'library.sol': 1
+    'library.sol': 1,
+    'transfer.sol': 1,
+    'ctor.sol': 0
   }
 
   runModuleOnFiles(module, t, (file, report) => {
@@ -214,7 +228,9 @@ test('Integration test similarVariableNames.js', function (t) {
     'structReentrant.sol': 0,
     'thisLocal.sol': 0,
     'globals.sol': 0,
-    'library.sol': 0
+    'library.sol': 0,
+    'transfer.sol': 0,
+    'ctor.sol': 1
   }
 
   runModuleOnFiles(module, t, (file, report) => {
@@ -241,7 +257,9 @@ test('Integration test inlineAssembly.js', function (t) {
     'structReentrant.sol': 0,
     'thisLocal.sol': 0,
     'globals.sol': 0,
-    'library.sol': 0
+    'library.sol': 0,
+    'transfer.sol': 0,
+    'ctor.sol': 0
   }
 
   runModuleOnFiles(module, t, (file, report) => {
@@ -268,7 +286,9 @@ test('Integration test blockTimestamp.js', function (t) {
     'structReentrant.sol': 0,
     'thisLocal.sol': 0,
     'globals.sol': 2,
-    'library.sol': 0
+    'library.sol': 0,
+    'transfer.sol': 0,
+    'ctor.sol': 0
   }
 
   runModuleOnFiles(module, t, (file, report) => {
@@ -295,7 +315,9 @@ test('Integration test lowLevelCalls.js', function (t) {
     'structReentrant.sol': 1,
     'thisLocal.sol': 2,
     'globals.sol': 1,
-    'library.sol': 1
+    'library.sol': 1,
+    'transfer.sol': 0,
+    'ctor.sol': 0
   }
 
   runModuleOnFiles(module, t, (file, report) => {
@@ -322,7 +344,9 @@ test('Integration test blockBlockhash.js', function (t) {
     'structReentrant.sol': 0,
     'thisLocal.sol': 0,
     'globals.sol': 1,
-    'library.sol': 0
+    'library.sol': 0,
+    'transfer.sol': 0,
+    'ctor.sol': 0
   }
 
   runModuleOnFiles(module, t, (file, report) => {

test/staticanalysis/test-contracts/ctor.sol

@@ -0,0 +1,8 @@
+contract c {
+    uint x;
+    uint x_abc;
+    function c(uint _x, uint _abc) {
+        x=_x;
+        x_abc=_abc;
+    }
+}

test/staticanalysis/test-contracts/transfer.sol

@@ -0,0 +1,7 @@
+contract c {
+    uint x;
+    function f(address r) {
+        r.transfer(1);
+        x = 2;
+    }
+}