updated logic with tests

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

@@ -9,12 +9,18 @@ function etherTransferInLoop () {
 
 etherTransferInLoop.prototype.visit = function (node) {
   if (common.isLoop(node)) {
-    var loopBlockStartIndex = common.getLoopBlockStartIndex()
-    var transferNodes = node.children[loopBlockStartIndex].children.filter(node => (common.isExpressionStatement(node) && common.isTransfer(node)))
+    var transferNodes = []
+    var loopBlockStartIndex = common.getLoopBlockStartIndex(node)
+    if (common.isBlock(node.children[loopBlockStartIndex])) {
+      transferNodes = node.children[loopBlockStartIndex].children
+      .filter(child => (common.isExpressionStatement(child) &&
+                    child.children[0].name === 'FunctionCall' &&
+                    common.isTransfer(child.children[0].children[0])))
       if (transferNodes.length > 0) {
         this.relevantNodes.push(...transferNodes)
       }
     }
+  }
 }
 
 etherTransferInLoop.prototype.report = function (compilationResults) {

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

@@ -1159,6 +1159,7 @@ module.exports = {
   isNewExpression: isNewExpression,
   isReturn: isReturn,
   isStatement: isStatement,
+  isExpressionStatement: isExpressionStatement,
   isBlock: isBlock,
 
   // #################### Constants

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

@@ -37,6 +37,7 @@ var testFiles = [
   'intDivisionTruncate.sol',
   'ERC20.sol',
   'stringBytesLength.sol',
+  'etherTransferInLoop.sol',
   'forLoopIteratesOverDynamicArray.sol'
 ]
 
@@ -77,6 +78,7 @@ test('Integration test thisLocal.js', function (t) {
     'intDivisionTruncate.sol': 0,
     'ERC20.sol': 0,
     'stringBytesLength.sol': 0,
+    'etherTransferInLoop.sol': 0,
     'forLoopIteratesOverDynamicArray.sol': 0
   }
 
@@ -115,6 +117,7 @@ test('Integration test checksEffectsInteraction.js', function (t) {
     'intDivisionTruncate.sol': 0,
     'ERC20.sol': 0,
     'stringBytesLength.sol': 0,
+    'etherTransferInLoop.sol': 0,
     'forLoopIteratesOverDynamicArray.sol': 0
   }
 
@@ -153,6 +156,7 @@ test('Integration test constantFunctions.js', function (t) {
     'intDivisionTruncate.sol': 0,
     'ERC20.sol': 0,
     'stringBytesLength.sol': 0,
+    'etherTransferInLoop.sol': 0,
     'forLoopIteratesOverDynamicArray.sol': 0
   }
 
@@ -191,6 +195,7 @@ test('Integration test inlineAssembly.js', function (t) {
     'intDivisionTruncate.sol': 0,
     'ERC20.sol': 0,
     'stringBytesLength.sol': 0,
+    'etherTransferInLoop.sol': 0,
     'forLoopIteratesOverDynamicArray.sol': 0
   }
 
@@ -229,6 +234,7 @@ test('Integration test txOrigin.js', function (t) {
     'intDivisionTruncate.sol': 0,
     'ERC20.sol': 0,
     'stringBytesLength.sol': 0,
+    'etherTransferInLoop.sol': 0,
     'forLoopIteratesOverDynamicArray.sol': 0
   }
 
@@ -267,6 +273,7 @@ test('Integration test gasCosts.js', function (t) {
     'intDivisionTruncate.sol': 1,
     'ERC20.sol': 2,
     'stringBytesLength.sol': 1,
+    'etherTransferInLoop.sol': 3,
     'forLoopIteratesOverDynamicArray.sol': 1
   }
 
@@ -305,6 +312,7 @@ test('Integration test similarVariableNames.js', function (t) {
     'intDivisionTruncate.sol': 0,
     'ERC20.sol': 0,
     'stringBytesLength.sol': 0,
+    'etherTransferInLoop.sol': 0,
     'forLoopIteratesOverDynamicArray.sol': 0
   }
 
@@ -343,6 +351,7 @@ test('Integration test inlineAssembly.js', function (t) {
     'intDivisionTruncate.sol': 0,
     'ERC20.sol': 0,
     'stringBytesLength.sol': 0,
+    'etherTransferInLoop.sol': 0,
     'forLoopIteratesOverDynamicArray.sol': 0
   }
 
@@ -381,6 +390,7 @@ test('Integration test blockTimestamp.js', function (t) {
     'intDivisionTruncate.sol': 0,
     'ERC20.sol': 0,
     'stringBytesLength.sol': 0,
+    'etherTransferInLoop.sol': 0,
     'forLoopIteratesOverDynamicArray.sol': 0
   }
 
@@ -419,6 +429,7 @@ test('Integration test lowLevelCalls.js', function (t) {
     'intDivisionTruncate.sol': 0,
     'ERC20.sol': 0,
     'stringBytesLength.sol': 0,
+    'etherTransferInLoop.sol': 0,
     'forLoopIteratesOverDynamicArray.sol': 0
   }
 
@@ -457,6 +468,7 @@ test('Integration test blockBlockhash.js', function (t) {
     'intDivisionTruncate.sol': 0,
     'ERC20.sol': 0,
     'stringBytesLength.sol': 0,
+    'etherTransferInLoop.sol': 0,
     'forLoopIteratesOverDynamicArray.sol': 0
   }
 
@@ -495,6 +507,7 @@ test('Integration test noReturn.js', function (t) {
     'intDivisionTruncate.sol': 0,
     'ERC20.sol': 0,
     'stringBytesLength.sol': 0,
+    'etherTransferInLoop.sol': 0,
     'forLoopIteratesOverDynamicArray.sol': 0
   }
 
@@ -533,6 +546,7 @@ test('Integration test selfdestruct.js', function (t) {
     'ERC20.sol': 0,
     'intDivisionTruncate.sol': 5,
     'stringBytesLength.sol': 0,
+    'etherTransferInLoop.sol': 0,
     'forLoopIteratesOverDynamicArray.sol': 0
   }
 
@@ -571,6 +585,7 @@ test('Integration test guardConditions.js', function (t) {
     'intDivisionTruncate.sol': 1,
     'ERC20.sol': 0,
     'stringBytesLength.sol': 0,
+    'etherTransferInLoop.sol': 0,
     'forLoopIteratesOverDynamicArray.sol': 0
   }
 
@@ -609,6 +624,7 @@ test('Integration test deleteDynamicArrays.js', function (t) {
     'intDivisionTruncate.sol': 0,
     'ERC20.sol': 0,
     'stringBytesLength.sol': 0,
+    'etherTransferInLoop.sol': 0,
     'forLoopIteratesOverDynamicArray.sol': 0
   }
 
@@ -647,6 +663,7 @@ test('Integration test deleteFromDynamicArray.js', function (t) {
     'intDivisionTruncate.sol': 0,
     'ERC20.sol': 0,
     'stringBytesLength.sol': 0,
+    'etherTransferInLoop.sol': 0,
     'forLoopIteratesOverDynamicArray.sol': 0
   }
 
@@ -685,6 +702,7 @@ test('Integration test assignAndCompare.js', function (t) {
     'intDivisionTruncate.sol': 0,
     'ERC20.sol': 0,
     'stringBytesLength.sol': 0,
+    'etherTransferInLoop.sol': 0,
     'forLoopIteratesOverDynamicArray.sol': 0
   }
 
@@ -723,6 +741,7 @@ test('Integration test intDivisionTruncate.js', function (t) {
     'intDivisionTruncate.sol': 2,
     'ERC20.sol': 0,
     'stringBytesLength.sol': 0,
+    'etherTransferInLoop.sol': 0,
     'forLoopIteratesOverDynamicArray.sol': 0
   }
 
@@ -761,6 +780,7 @@ test('Integration test erc20Decimal.js', function (t) {
     'intDivisionTruncate.sol': 0,
     'ERC20.sol': 1,
     'stringBytesLength.sol': 0,
+    'etherTransferInLoop.sol': 0,
     'forLoopIteratesOverDynamicArray.sol': 0
   }
 
@@ -799,6 +819,7 @@ test('Integration test stringBytesLength.js', function (t) {
     'intDivisionTruncate.sol': 0,
     'ERC20.sol': 0,
     'stringBytesLength.sol': 1,
+    'etherTransferInLoop.sol': 0,
     'forLoopIteratesOverDynamicArray.sol': 0
   }
 
@@ -807,6 +828,45 @@ test('Integration test stringBytesLength.js', function (t) {
   })
 })
 
+test('Integration test etherTransferInLoop.js', function (t) {
+  t.plan(testFiles.length)
+
+  var module = require('../../src/solidity-analyzer/modules/etherTransferInLoop')
+
+  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': 0,
+    'library.sol': 0,
+    'transfer.sol': 0,
+    'ctor.sol': 0,
+    'forgottenReturn.sol': 0,
+    'selfdestruct.sol': 0,
+    'deleteDynamicArray.sol': 0,
+    'deleteFromDynamicArray.sol': 0,
+    'blockLevelCompare.sol': 0,
+    'intDivisionTruncate.sol': 0,
+    'ERC20.sol': 0,
+    'stringBytesLength.sol': 0,
+    'etherTransferInLoop.sol': 3,
+    'forLoopIteratesOverDynamicArray.sol': 0
+  }
+
+  runModuleOnFiles(module, t, (file, report) => {
+    t.equal(report.length, lengthCheck[file], `${file} has right amount of etherTransferInLoop warnings`)
+  })
+})
+
 test('Integration test forLoopIteratesOverDynamicArray.js', function (t) {
   t.plan(testFiles.length)
 
@@ -837,6 +897,7 @@ test('Integration test forLoopIteratesOverDynamicArray.js', function (t) {
     'intDivisionTruncate.sol': 0,
     'ERC20.sol': 0,
     'stringBytesLength.sol': 0,
+    'etherTransferInLoop.sol': 0,
     'forLoopIteratesOverDynamicArray.sol': 1
   }
 

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

@@ -37,6 +37,7 @@ var testFiles = [
   'intDivisionTruncate.sol',
   'ERC20.sol',
   'stringBytesLength.sol',
+  'etherTransferInLoop.sol',
   'forLoopIteratesOverDynamicArray.sol'
 ]
 
@@ -77,6 +78,7 @@ test('Integration test thisLocal.js', function (t) {
     'intDivisionTruncate.sol': 0,
     'ERC20.sol': 0,
     'stringBytesLength.sol': 0,
+    'etherTransferInLoop.sol': 0,
     'forLoopIteratesOverDynamicArray.sol': 0
   }
 
@@ -115,6 +117,7 @@ test('Integration test checksEffectsInteraction.js', function (t) {
     'intDivisionTruncate.sol': 0,
     'ERC20.sol': 0,
     'stringBytesLength.sol': 0,
+    'etherTransferInLoop.sol': 0,
     'forLoopIteratesOverDynamicArray.sol': 0
   }
 
@@ -153,6 +156,7 @@ test('Integration test constantFunctions.js', function (t) {
     'intDivisionTruncate.sol': 0,
     'ERC20.sol': 0,
     'stringBytesLength.sol': 0,
+    'etherTransferInLoop.sol': 0,
     'forLoopIteratesOverDynamicArray.sol': 0
   }
 
@@ -191,6 +195,7 @@ test('Integration test inlineAssembly.js', function (t) {
     'intDivisionTruncate.sol': 0,
     'ERC20.sol': 0,
     'stringBytesLength.sol': 0,
+    'etherTransferInLoop.sol': 0,
     'forLoopIteratesOverDynamicArray.sol': 0
   }
 
@@ -229,6 +234,7 @@ test('Integration test txOrigin.js', function (t) {
     'intDivisionTruncate.sol': 0,
     'ERC20.sol': 0,
     'stringBytesLength.sol': 0,
+    'etherTransferInLoop.sol': 0,
     'forLoopIteratesOverDynamicArray.sol': 0
   }
 
@@ -267,6 +273,7 @@ test('Integration test gasCosts.js', function (t) {
     'intDivisionTruncate.sol': 1,
     'ERC20.sol': 2,
     'stringBytesLength.sol': 1,
+    'etherTransferInLoop.sol': 3,
     'forLoopIteratesOverDynamicArray.sol': 1
   }
 
@@ -305,6 +312,7 @@ test('Integration test similarVariableNames.js', function (t) {
     'intDivisionTruncate.sol': 0,
     'ERC20.sol': 0,
     'stringBytesLength.sol': 0,
+    'etherTransferInLoop.sol': 0,
     'forLoopIteratesOverDynamicArray.sol': 0
   }
 
@@ -343,6 +351,7 @@ test('Integration test inlineAssembly.js', function (t) {
     'intDivisionTruncate.sol': 0,
     'ERC20.sol': 0,
     'stringBytesLength.sol': 0,
+    'etherTransferInLoop.sol': 0,
     'forLoopIteratesOverDynamicArray.sol': 0
   }
 
@@ -381,6 +390,7 @@ test('Integration test blockTimestamp.js', function (t) {
     'intDivisionTruncate.sol': 0,
     'ERC20.sol': 0,
     'stringBytesLength.sol': 0,
+    'etherTransferInLoop.sol': 0,
     'forLoopIteratesOverDynamicArray.sol': 0
   }
 
@@ -419,6 +429,7 @@ test('Integration test lowLevelCalls.js', function (t) {
     'intDivisionTruncate.sol': 0,
     'ERC20.sol': 0,
     'stringBytesLength.sol': 0,
+    'etherTransferInLoop.sol': 0,
     'forLoopIteratesOverDynamicArray.sol': 0
   }
 
@@ -457,6 +468,7 @@ test('Integration test blockBlockhash.js', function (t) {
     'intDivisionTruncate.sol': 0,
     'ERC20.sol': 0,
     'stringBytesLength.sol': 0,
+    'etherTransferInLoop.sol': 0,
     'forLoopIteratesOverDynamicArray.sol': 0
   }
 
@@ -538,6 +550,7 @@ test('Integration test selfdestruct.js', function (t) {
     'ERC20.sol': 0,
     'intDivisionTruncate.sol': 5,
     'stringBytesLength.sol': 0,
+    'etherTransferInLoop.sol': 0,
     'forLoopIteratesOverDynamicArray.sol': 0
   }
 
@@ -576,6 +589,7 @@ test('Integration test guardConditions.js', function (t) {
     'intDivisionTruncate.sol': 1,
     'ERC20.sol': 0,
     'stringBytesLength.sol': 0,
+    'etherTransferInLoop.sol': 0,
     'forLoopIteratesOverDynamicArray.sol': 0
   }
 
@@ -614,6 +628,7 @@ test('Integration test deleteDynamicArrays.js', function (t) {
     'intDivisionTruncate.sol': 0,
     'ERC20.sol': 0,
     'stringBytesLength.sol': 0,
+    'etherTransferInLoop.sol': 0,
     'forLoopIteratesOverDynamicArray.sol': 0
   }
 
@@ -652,6 +667,7 @@ test('Integration test deleteFromDynamicArray.js', function (t) {
     'intDivisionTruncate.sol': 0,
     'ERC20.sol': 0,
     'stringBytesLength.sol': 0,
+    'etherTransferInLoop.sol': 0,
     'forLoopIteratesOverDynamicArray.sol': 0
   }
 
@@ -690,6 +706,7 @@ test('Integration test assignAndCompare.js', function (t) {
     'intDivisionTruncate.sol': 0,
     'ERC20.sol': 0,
     'stringBytesLength.sol': 0,
+    'etherTransferInLoop.sol': 0,
     'forLoopIteratesOverDynamicArray.sol': 0
   }
 
@@ -728,6 +745,7 @@ test('Integration test intDivisionTruncate.js', function (t) {
     'intDivisionTruncate.sol': 2,
     'ERC20.sol': 0,
     'stringBytesLength.sol': 0,
+    'etherTransferInLoop.sol': 0,
     'forLoopIteratesOverDynamicArray.sol': 0
   }
 
@@ -766,6 +784,7 @@ test('Integration test erc20Decimal.js', function (t) {
     'intDivisionTruncate.sol': 0,
     'ERC20.sol': 1,
     'stringBytesLength.sol': 0,
+    'etherTransferInLoop.sol': 0,
     'forLoopIteratesOverDynamicArray.sol': 0
   }
 
@@ -804,6 +823,7 @@ test('Integration test stringBytesLength.js', function (t) {
     'intDivisionTruncate.sol': 0,
     'ERC20.sol': 0,
     'stringBytesLength.sol': 1,
+    'etherTransferInLoop.sol': 0,
     'forLoopIteratesOverDynamicArray.sol': 0
   }
 
@@ -812,6 +832,45 @@ test('Integration test stringBytesLength.js', function (t) {
   })
 })
 
+test('Integration test etherTransferInLoop.js', function (t) {
+  t.plan(testFiles.length)
+
+  var module = require('../../src/solidity-analyzer/modules/etherTransferInLoop')
+
+  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': 0,
+    'library.sol': 0,
+    'transfer.sol': 0,
+    'ctor.sol': 0,
+    'forgottenReturn.sol': 0,
+    'selfdestruct.sol': 0,
+    'deleteDynamicArray.sol': 0,
+    'deleteFromDynamicArray.sol': 0,
+    'blockLevelCompare.sol': 0,
+    'intDivisionTruncate.sol': 0,
+    'ERC20.sol': 0,
+    'stringBytesLength.sol': 0,
+    'etherTransferInLoop.sol': 3,
+    'forLoopIteratesOverDynamicArray.sol': 0
+  }
+
+  runModuleOnFiles(module, t, (file, report) => {
+    t.equal(report.length, lengthCheck[file], `${file} has right amount of etherTransferInLoop warnings`)
+  })
+})
+
 test('Integration test forLoopIteratesOverDynamicArray.js', function (t) {
   t.plan(testFiles.length)
 
@@ -842,6 +901,7 @@ test('Integration test forLoopIteratesOverDynamicArray.js', function (t) {
     'intDivisionTruncate.sol': 0,
     'ERC20.sol': 0,
     'stringBytesLength.sol': 0,
+    'etherTransferInLoop.sol': 0,
     'forLoopIteratesOverDynamicArray.sol': 1
   }
 

remix-analyzer/test/analysis/test-contracts/solidity-v0.4.24/etherTransferInLoop.sol

+pragma solidity ^0.4.24;
+
+contract etherTransferInLoop {
+    address owner;
+
+    constructor() public {
+        owner = msg.sender;
+    }
+
+    function transferInForLoop(uint index) public  {
+        for (uint i = index; i < 10; i++) {
+            owner.transfer(i);
+        }
+    }
+
+    function transferInWhileLoop(uint index) public  {
+        uint i = index;
+        while (i < 10) {
+            owner.transfer(i);
+            i++;
+        }
+    }
+
+    function transferInDoWhileLoop(uint index) public  {
+        uint i = index;
+        do {
+            owner.transfer(i);
+            i++;
+        } while (i < 10);
+    }
+}
\ No newline at end of file

remix-analyzer/test/analysis/test-contracts/solidity-v0.4.24/loops.sol

+pragma solidity ^0.4.9;
+
+contract loops {
+    uint[] array;
+    constructor(uint[] memory _array) public {
+        array = _array;
+    }
+
+    function fnWithForLoop(uint index) public  {
+        for (uint i = index; i < 10; i++) {
+            array.push(i);
+        }
+    }
+
+    function fnWithWhileLoop(uint index) public  {
+        uint i = index;
+        while (i < 10) {
+            array.push(i);
+            i++;
+        }
+    }
+
+    function fnWithDoWhileLoop(uint index) public  {
+        uint i = index;
+        do{
+            array.push(i);
+            i++;
+        }while (i < 10);
+    }
+}
\ No newline at end of file

remix-analyzer/test/analysis/test-contracts/solidity-v0.5/etherTransferInLoop.sol

+pragma solidity >=0.4.9 <0.6.0;
+
+contract etherTransferInLoop {
+    address payable owner;
+
+    constructor() public {
+        owner = msg.sender;
+    }
+
+    function transferInForLoop(uint index) public  {
+        for (uint i = index; i < 10; i++) {
+            owner.transfer(i);
+        }
+    }
+
+    function transferInWhileLoop(uint index) public  {
+        uint i = index;
+        while (i < 10) {
+            owner.transfer(i);
+            i++;
+        }
+    }
+
+    function transferInDoWhileLoop(uint index) public  {
+        uint i = index;
+        do {
+            owner.transfer(i);
+            i++;
+        } while (i < 10);
+    }
+}
\ No newline at end of file

remix-analyzer/test/analysis/test-contracts/solidity-v0.5/loops.sol

+pragma solidity >=0.4.9 <0.6.0;
+
+contract loops {
+    uint[] array;
+    constructor(uint[] memory _array) public {
+        array = _array;
+    }
+
+    function fnWithForLoop(uint index) public  {
+        for (uint i = index; i < 10; i++) {
+            array.push(i);
+        }
+    }
+
+    function fnWithWhileLoop(uint index) public  {
+        uint i = index;
+        while (i < 10) {
+            array.push(i);
+            i++;
+        }
+    }
+
+    function fnWithDoWhileLoop(uint index) public  {
+        uint i = index;
+        do{
+            array.push(i);
+            i++;
+        }while (i < 10);
+    }
+}
\ No newline at end of file