Static analysis: Add module for truncation int/uint division (Issue #948)

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

@@ -0,0 +1,28 @@
+var name = 'Data Trucated: '
+var desc = 'Division on int/uint values truncates the result.'
+var categories = require('./categories')
+var common = require('./staticAnalysisCommon')
+
+function intDivitionTruncate () {
+  this.warningNodes = []
+}
+
+intDivitionTruncate.prototype.visit = function (node) {
+  if (common.isIntDivision(node)) this.warningNodes.push(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.',
+      location: item.src
+    }
+  })
+}
+
+module.exports = {
+  name: name,
+  description: desc,
+  category: categories.MISC,
+  Module: intDivitionTruncate
+}

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

@@ -615,6 +615,15 @@ function isConstructor (node) {
   )
 }
 
+/**
+ * True if node is integer division that truncates (not only int literals since those yield a rational value)
+ * @node {ASTNode} some AstNode
+ * @return {bool}
+ */
+function isIntDivision (node) {
+  return isBinaryOperation(node) && operator(node, exactMatch(util.escapeRegExp('/'))) && expressionType(node, util.escapeRegExp('int'))
+}
+
 /**
  * True if is block / SubScope has top level binops (e.g. that are not assigned to anything, most of the time confused compare instead of assign)
  * @node {ASTNode} some AstNode
@@ -982,6 +991,7 @@ module.exports = {
   isSelfdestructCall: isSelfdestructCall,
   isAssertCall: isAssertCall,
   isRequireCall: isRequireCall,
+  isIntDivision: isIntDivision,
 
   // #################### Trivial Node Identification
   isDeleteUnaryOperation: isDeleteUnaryOperation,

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

@@ -29,7 +29,8 @@ var testFiles = [
   'forgottenReturn.sol',
   'selfdestruct.sol',
   'deleteDynamicArray.sol',
-  'blockLevelCompare.sol'
+  'blockLevelCompare.sol',
+  'intDivisionTruncate.sol'
 ]
 
 var testFileAsts = {}
@@ -64,7 +65,8 @@ test('Integration test thisLocal.js', function (t) {
     'forgottenReturn.sol': 0,
     'selfdestruct.sol': 0,
     'deleteDynamicArray.sol': 0,
-    'blockLevelCompare.sol': 0
+    'blockLevelCompare.sol': 0,
+    'intDivisionTruncate.sol': 0
   }
 
   runModuleOnFiles(module, t, (file, report) => {
@@ -97,7 +99,8 @@ test('Integration test checksEffectsInteraction.js', function (t) {
     'forgottenReturn.sol': 0,
     'selfdestruct.sol': 0,
     'deleteDynamicArray.sol': 0,
-    'blockLevelCompare.sol': 0
+    'blockLevelCompare.sol': 0,
+    'intDivisionTruncate.sol': 0
   }
 
   runModuleOnFiles(module, t, (file, report) => {
@@ -130,7 +133,8 @@ test('Integration test constantFunctions.js', function (t) {
     'forgottenReturn.sol': 0,
     'selfdestruct.sol': 1,
     'deleteDynamicArray.sol': 0,
-    'blockLevelCompare.sol': 0
+    'blockLevelCompare.sol': 0,
+    'intDivisionTruncate.sol': 0
   }
 
   runModuleOnFiles(module, t, (file, report) => {
@@ -163,7 +167,8 @@ test('Integration test inlineAssembly.js', function (t) {
     'forgottenReturn.sol': 0,
     'selfdestruct.sol': 0,
     'deleteDynamicArray.sol': 0,
-    'blockLevelCompare.sol': 0
+    'blockLevelCompare.sol': 0,
+    'intDivisionTruncate.sol': 0
   }
 
   runModuleOnFiles(module, t, (file, report) => {
@@ -196,7 +201,8 @@ test('Integration test txOrigin.js', function (t) {
     'forgottenReturn.sol': 0,
     'selfdestruct.sol': 0,
     'deleteDynamicArray.sol': 0,
-    'blockLevelCompare.sol': 0
+    'blockLevelCompare.sol': 0,
+    'intDivisionTruncate.sol': 0
   }
 
   runModuleOnFiles(module, t, (file, report) => {
@@ -229,7 +235,8 @@ test('Integration test gasCosts.js', function (t) {
     'forgottenReturn.sol': 3,
     'selfdestruct.sol': 0,
     'deleteDynamicArray.sol': 2,
-    'blockLevelCompare.sol': 1
+    'blockLevelCompare.sol': 1,
+    'intDivisionTruncate.sol': 1
   }
 
   runModuleOnFiles(module, t, (file, report) => {
@@ -262,7 +269,8 @@ test('Integration test similarVariableNames.js', function (t) {
     'forgottenReturn.sol': 0,
     'selfdestruct.sol': 0,
     'deleteDynamicArray.sol': 1,
-    'blockLevelCompare.sol': 0
+    'blockLevelCompare.sol': 0,
+    'intDivisionTruncate.sol': 0
   }
 
   runModuleOnFiles(module, t, (file, report) => {
@@ -295,7 +303,8 @@ test('Integration test inlineAssembly.js', function (t) {
     'forgottenReturn.sol': 0,
     'selfdestruct.sol': 0,
     'deleteDynamicArray.sol': 0,
-    'blockLevelCompare.sol': 0
+    'blockLevelCompare.sol': 0,
+    'intDivisionTruncate.sol': 0
   }
 
   runModuleOnFiles(module, t, (file, report) => {
@@ -328,7 +337,8 @@ test('Integration test blockTimestamp.js', function (t) {
     'forgottenReturn.sol': 0,
     'selfdestruct.sol': 0,
     'deleteDynamicArray.sol': 0,
-    'blockLevelCompare.sol': 0
+    'blockLevelCompare.sol': 0,
+    'intDivisionTruncate.sol': 0
   }
 
   runModuleOnFiles(module, t, (file, report) => {
@@ -361,7 +371,8 @@ test('Integration test lowLevelCalls.js', function (t) {
     'forgottenReturn.sol': 0,
     'selfdestruct.sol': 0,
     'deleteDynamicArray.sol': 0,
-    'blockLevelCompare.sol': 0
+    'blockLevelCompare.sol': 0,
+    'intDivisionTruncate.sol': 0
   }
 
   runModuleOnFiles(module, t, (file, report) => {
@@ -394,7 +405,8 @@ test('Integration test blockBlockhash.js', function (t) {
     'forgottenReturn.sol': 0,
     'selfdestruct.sol': 0,
     'deleteDynamicArray.sol': 0,
-    'blockLevelCompare.sol': 0
+    'blockLevelCompare.sol': 0,
+    'intDivisionTruncate.sol': 0
   }
 
   runModuleOnFiles(module, t, (file, report) => {
@@ -427,7 +439,8 @@ test('Integration test noReturn.js', function (t) {
     'forgottenReturn.sol': 1,
     'selfdestruct.sol': 0,
     'deleteDynamicArray.sol': 0,
-    'blockLevelCompare.sol': 0
+    'blockLevelCompare.sol': 0,
+    'intDivisionTruncate.sol': 0
   }
 
   runModuleOnFiles(module, t, (file, report) => {
@@ -460,7 +473,8 @@ test('Integration test selfdestruct.js', function (t) {
     'forgottenReturn.sol': 0,
     'selfdestruct.sol': 2,
     'deleteDynamicArray.sol': 0,
-    'blockLevelCompare.sol': 0
+    'blockLevelCompare.sol': 0,
+    'intDivisionTruncate.sol': 1
   }
 
   runModuleOnFiles(module, t, (file, report) => {
@@ -493,7 +507,8 @@ test('Integration test guardConditions.js', function (t) {
     'forgottenReturn.sol': 0,
     'selfdestruct.sol': 0,
     'deleteDynamicArray.sol': 1,
-    'blockLevelCompare.sol': 0
+    'blockLevelCompare.sol': 0,
+    'intDivisionTruncate.sol': 1
   }
 
   runModuleOnFiles(module, t, (file, report) => {
@@ -526,7 +541,8 @@ test('Integration test deleteDynamicArrays.js', function (t) {
     'forgottenReturn.sol': 0,
     'selfdestruct.sol': 0,
     'deleteDynamicArray.sol': 2,
-    'blockLevelCompare.sol': 0
+    'blockLevelCompare.sol': 0,
+    'intDivisionTruncate.sol': 0
   }
 
   runModuleOnFiles(module, t, (file, report) => {
@@ -559,7 +575,8 @@ test('Integration test assignAndCompare.js', function (t) {
     'forgottenReturn.sol': 0,
     'selfdestruct.sol': 0,
     'deleteDynamicArray.sol': 0,
-    'blockLevelCompare.sol': 8
+    'blockLevelCompare.sol': 8,
+    'intDivisionTruncate.sol': 0
   }
 
   runModuleOnFiles(module, t, (file, report) => {
@@ -567,6 +584,40 @@ test('Integration test assignAndCompare.js', function (t) {
   })
 })
 
+test('Integration test intDivisionTruncate.js', function (t) {
+  t.plan(testFiles.length)
+
+  var module = require('../../src/analysis/modules/intDivisionTruncate')
+
+  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,
+    'blockLevelCompare.sol': 0,
+    'intDivisionTruncate.sol': 2
+  }
+
+  runModuleOnFiles(module, t, (file, report) => {
+    t.equal(report.length, lengthCheck[file], `${file} has right amount of intDivisionTruncate warnings`)
+  })
+})
+
 // #################### Helpers
 function runModuleOnFiles (module, t, cb) {
   var statRunner = new StatRunner()

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

@@ -0,0 +1,28 @@
+pragma solidity ^0.4.19;
+
+contract CharityCampaign {
+    mapping (address => uint) contributions; 
+    int128 feePercentage;
+    uint p2;
+    address processor;
+    address beneficiary;
+
+    function CharityCampaign(address _beneficiary, int128 _feePercentage) public {
+        processor = msg.sender;
+        beneficiary = _beneficiary;
+        feePercentage = _feePercentage;
+    }
+
+    function contribute() payable public returns (uint feeCollected) {
+        uint fee = msg.value * uint256(feePercentage / 100);
+        fee = msg.value * (p2 / 100);
+        contributions[msg.sender] = msg.value - fee;
+        processor.transfer(fee);
+        return fee;
+    }
+
+    function endCampaign() public {
+        require(msg.sender == processor || msg.sender == beneficiary);
+        selfdestruct(beneficiary);
+    }
+}