Merge branch 'master' into bytesStringLength

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

@@ -31,6 +31,7 @@ function abstractAstView () {
  *      "modifierInvocations": [], // Modifier invocation AST nodes that are applied on this function
  *      "localVariables": [],      // Local variable declaration nodes
  *      "parameters": []           // Parameter types of the function in order of definition
+ *      "returns": []              // list of return vars as { type: ... , name: ... }
  *    }
  *  ],
  *  "modifiers": [],              // Modifiers definded by the contract, format similar to functions

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

@@ -1,5 +1,6 @@
 module.exports = {
   SECURITY: {displayName: 'Security', id: 'SEC'},
   GAS: {displayName: 'Gas & Economy', id: 'GAS'},
-  MISC: {displayName: 'Miscellaneous', id: 'MISC'}
+  MISC: {displayName: 'Miscellaneous', id: 'MISC'},
+  ERC: {displayName: 'ERC', id: 'ERC'}
 }

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

@@ -0,0 +1,63 @@
+var name = 'ERC20: '
+var desc = 'Decimal should be uint8'
+var categories = require('./categories')
+var common = require('./staticAnalysisCommon')
+var AbstractAst = require('./abstractAstView')
+var algo = require('./algorithmCategories')
+
+function erc20Decimals () {
+  this.abstractAst = new AbstractAst()
+  this.visit = this.abstractAst.build_visit(
+    (node) => false
+  )
+  this.report = this.abstractAst.build_report(report)
+}
+
+erc20Decimals.prototype.visit = function () { throw new Error('erc20Decimals.js no visit function set upon construction') }
+
+erc20Decimals.prototype.report = function () { throw new Error('erc20Decimals.js no report function set upon construction') }
+
+function report (contracts, multipleContractsWithSameName) {
+  var warnings = []
+
+  contracts.forEach((contract) => {
+    let contractAbiSignatures = contract.functions.map((f) => common.helpers.buildAbiSignature(common.getFunctionDefinitionName(f.node), f.parameters))
+
+    if (isERC20(contractAbiSignatures)) {
+      let decimalsVar = contract.stateVariables.filter((stateVar) => common.getDeclaredVariableName(stateVar) === 'decimals' && (common.getDeclaredVariableType(stateVar) !== 'uint8' || stateVar.attributes.visibility !== 'public'))
+      let decimalsFun = contract.functions.filter((f) => common.getFunctionDefinitionName(f.node) === 'decimals' &&
+                                                          (
+                                                            (f.returns.length === 0 || f.returns.length > 1) ||
+                                                            (f.returns.length === 1 && (f.returns[0].type !== 'uint8' || f.node.attributes.visibility !== 'public'))
+                                                          )
+                                                  )
+
+      if (decimalsVar.length > 0 || decimalsFun.length > 0) {
+        warnings.push({
+          warning: 'ERC20 Contracts decimals function should have uint8 as return type',
+          location: null,
+          more: ' https://eips.ethereum.org/EIPS/eip-20'
+        })
+      }
+    }
+  })
+
+  return warnings
+}
+
+function isERC20 (funSignatures) {
+  return funSignatures.includes('totalSupply()') &&
+         funSignatures.includes('balanceOf(address)') &&
+         funSignatures.includes('transfer(address,uint256)') &&
+         funSignatures.includes('transferFrom(address,address,uint256)') &&
+         funSignatures.includes('approve(address,uint256)') &&
+         funSignatures.includes('allowance(address,address)')
+}
+
+module.exports = {
+  name: name,
+  description: desc,
+  category: categories.ERC,
+  algorithm: algo.EXACT,
+  Module: erc20Decimals
+}

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

@@ -14,5 +14,6 @@ module.exports = [
   require('./guardConditions'),
   require('./deleteDynamicArrays'),
   require('./assignAndCompare'),
+  require('./erc20Decimals'),
   require('./stringBytesLength')
 ]

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

@@ -266,6 +266,18 @@ function getDeclaredVariableName (varDeclNode) {
   return varDeclNode.attributes.name
 }
 
+/**
+ * Returns the type of a variable definition, Throws on wrong node.
+ * Example:
+ *   var x = 10; => x
+ * @varDeclNode {ASTNode} Variable declaration node
+ * @return {string} variable type
+ */
+function getDeclaredVariableType (varDeclNode) {
+  if (!isVariableDeclaration(varDeclNode)) throw new Error('staticAnalysisCommon.js: not a variable declaration')
+  return varDeclNode.attributes.type
+}
+
 /**
  * Returns state variable declaration nodes for a contract, Throws on wrong node.
  * Example:
@@ -758,7 +770,7 @@ function isBlockTimestampAccess (node) {
  * @return {bool}
  */
 function isBlockBlockHashAccess (node) {
-  return isSpecialVariableAccess(node, specialVariables.BLOCKHASH)
+  return isSpecialVariableAccess(node, specialVariables.BLOCKHASH) || isBuiltinFunctionCall(node) && getLocalCallName(node) === 'blockhash'
 }
 
 /**
@@ -934,6 +946,10 @@ function buildFunctionSignature (paramTypes, returnTypes, isPayable, additionalM
   return 'function (' + util.concatWithSeperator(paramTypes, ',') + ')' + ((isPayable) ? ' payable' : '') + ((additionalMods) ? ' ' + additionalMods : '') + ((returnTypes.length) ? ' returns (' + util.concatWithSeperator(returnTypes, ',') + ')' : '')
 }
 
+function buildAbiSignature (funName, paramTypes) {
+  return funName + '(' + util.concatWithSeperator(paramTypes, ',') + ')'
+}
+
 /**
  * Finds first node of a certain type under a specific node.
  * @node {AstNode} node to start form
@@ -963,6 +979,7 @@ module.exports = {
   getContractName: getContractName,
   getEffectedVariableName: getEffectedVariableName,
   getDeclaredVariableName: getDeclaredVariableName,
+  getDeclaredVariableType: getDeclaredVariableType,
   getLocalCallName: getLocalCallName,
   getInheritsFromName: getInheritsFromName,
   getExternalDirectCallContractName: getExternalDirectCallContractName,
@@ -1050,6 +1067,7 @@ module.exports = {
     nodeType: nodeType,
     name: name,
     operator: operator,
-    buildFunctionSignature: buildFunctionSignature
+    buildFunctionSignature: buildFunctionSignature,
+    buildAbiSignature: buildAbiSignature
   }
 }

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

@@ -31,6 +31,7 @@ var testFiles = [
   'deleteDynamicArray.sol',
   'blockLevelCompare.sol',
   'intDivisionTruncate.sol',
+  'ERC20.sol',
   'stringBytesLength.sol'
 ]
 
@@ -68,6 +69,7 @@ test('Integration test thisLocal.js', function (t) {
     'deleteDynamicArray.sol': 0,
     'blockLevelCompare.sol': 0,
     'intDivisionTruncate.sol': 0,
+    'ERC20.sol': 0,
     'stringBytesLength.sol': 0
   }
 
@@ -103,6 +105,7 @@ test('Integration test checksEffectsInteraction.js', function (t) {
     'deleteDynamicArray.sol': 0,
     'blockLevelCompare.sol': 0,
     'intDivisionTruncate.sol': 0,
+    'ERC20.sol': 0,
     'stringBytesLength.sol': 0
   }
 
@@ -138,6 +141,7 @@ test('Integration test constantFunctions.js', function (t) {
     'deleteDynamicArray.sol': 0,
     'blockLevelCompare.sol': 0,
     'intDivisionTruncate.sol': 0,
+    'ERC20.sol': 0,
     'stringBytesLength.sol': 0
   }
 
@@ -173,6 +177,7 @@ test('Integration test inlineAssembly.js', function (t) {
     'deleteDynamicArray.sol': 0,
     'blockLevelCompare.sol': 0,
     'intDivisionTruncate.sol': 0,
+    'ERC20.sol': 0,
     'stringBytesLength.sol': 0
   }
 
@@ -208,6 +213,7 @@ test('Integration test txOrigin.js', function (t) {
     'deleteDynamicArray.sol': 0,
     'blockLevelCompare.sol': 0,
     'intDivisionTruncate.sol': 0,
+    'ERC20.sol': 0,
     'stringBytesLength.sol': 0
   }
 
@@ -243,6 +249,7 @@ test('Integration test gasCosts.js', function (t) {
     'deleteDynamicArray.sol': 2,
     'blockLevelCompare.sol': 1,
     'intDivisionTruncate.sol': 1,
+    'ERC20.sol': 2,
     'stringBytesLength.sol': 1
   }
 
@@ -278,6 +285,7 @@ test('Integration test similarVariableNames.js', function (t) {
     'deleteDynamicArray.sol': 1,
     'blockLevelCompare.sol': 0,
     'intDivisionTruncate.sol': 0,
+    'ERC20.sol': 0,
     'stringBytesLength.sol': 0
   }
 
@@ -313,6 +321,7 @@ test('Integration test inlineAssembly.js', function (t) {
     'deleteDynamicArray.sol': 0,
     'blockLevelCompare.sol': 0,
     'intDivisionTruncate.sol': 0,
+    'ERC20.sol': 0,
     'stringBytesLength.sol': 0
   }
 
@@ -348,6 +357,7 @@ test('Integration test blockTimestamp.js', function (t) {
     'deleteDynamicArray.sol': 0,
     'blockLevelCompare.sol': 0,
     'intDivisionTruncate.sol': 0,
+    'ERC20.sol': 0,
     'stringBytesLength.sol': 0
   }
 
@@ -383,6 +393,7 @@ test('Integration test lowLevelCalls.js', function (t) {
     'deleteDynamicArray.sol': 0,
     'blockLevelCompare.sol': 0,
     'intDivisionTruncate.sol': 0,
+    'ERC20.sol': 0,
     'stringBytesLength.sol': 0
   }
 
@@ -418,6 +429,7 @@ test('Integration test blockBlockhash.js', function (t) {
     'deleteDynamicArray.sol': 0,
     'blockLevelCompare.sol': 0,
     'intDivisionTruncate.sol': 0,
+    'ERC20.sol': 0,
     'stringBytesLength.sol': 0
   }
 
@@ -453,6 +465,7 @@ test('Integration test noReturn.js', function (t) {
     'deleteDynamicArray.sol': 0,
     'blockLevelCompare.sol': 0,
     'intDivisionTruncate.sol': 0,
+    'ERC20.sol': 0,
     'stringBytesLength.sol': 0
   }
 
@@ -487,6 +500,7 @@ test('Integration test selfdestruct.js', function (t) {
     'selfdestruct.sol': 3,
     'deleteDynamicArray.sol': 0,
     'blockLevelCompare.sol': 0,
+    'ERC20.sol': 0,
     'intDivisionTruncate.sol': 5,
     'stringBytesLength.sol': 0
   }
@@ -523,6 +537,7 @@ test('Integration test guardConditions.js', function (t) {
     'deleteDynamicArray.sol': 1,
     'blockLevelCompare.sol': 0,
     'intDivisionTruncate.sol': 1,
+    'ERC20.sol': 0,
     'stringBytesLength.sol': 0
   }
 
@@ -558,6 +573,7 @@ test('Integration test deleteDynamicArrays.js', function (t) {
     'deleteDynamicArray.sol': 2,
     'blockLevelCompare.sol': 0,
     'intDivisionTruncate.sol': 0,
+    'ERC20.sol': 0,
     'stringBytesLength.sol': 0
   }
 
@@ -593,6 +609,7 @@ test('Integration test assignAndCompare.js', function (t) {
     'deleteDynamicArray.sol': 0,
     'blockLevelCompare.sol': 8,
     'intDivisionTruncate.sol': 0,
+    'ERC20.sol': 0,
     'stringBytesLength.sol': 0
   }
 
@@ -628,6 +645,7 @@ test('Integration test intDivisionTruncate.js', function (t) {
     'deleteDynamicArray.sol': 0,
     'blockLevelCompare.sol': 0,
     'intDivisionTruncate.sol': 2,
+    'ERC20.sol': 0,
     'stringBytesLength.sol': 0
   }
 
@@ -636,6 +654,42 @@ test('Integration test intDivisionTruncate.js', function (t) {
   })
 })
 
+test('Integration test erc20Decimal.js', function (t) {
+  t.plan(testFiles.length)
+
+  var module = require('../../src/solidity-analyzer/modules/erc20Decimals')
+
+  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': 0,
+    'ERC20.sol': 1,
+    'stringBytesLength.sol': 0
+  }
+
+  runModuleOnFiles(module, t, (file, report) => {
+    t.equal(report.length, lengthCheck[file], `${file} has right amount of erc20Decimals warnings`)
+  })
+})
+
 test('Integration test stringBytesLength.js', function (t) {
   t.plan(testFiles.length)
 
@@ -663,6 +717,7 @@ test('Integration test stringBytesLength.js', function (t) {
     'deleteDynamicArray.sol': 0,
     'blockLevelCompare.sol': 0,
     'intDivisionTruncate.sol': 0,
+    'ERC20.sol': 0,
     'stringBytesLength.sol': 1
   }
 

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

@@ -0,0 +1,46 @@
+
+pragma solidity ^0.4.17;
+contract EIP20 {
+
+    uint public decimals = 12;
+    
+    // optional
+    function name() public pure returns (string) {
+        return "MYTOKEN";
+    }
+    
+    // optional
+    function symbol() public pure returns (string) {
+       return "MT";
+    }
+    
+    // optional
+    //function decimals() internal pure returns (uint8) {
+    //   return 12;
+    //}
+    
+    function totalSupply() public pure returns (uint256) {
+        return 12000;
+    }
+    
+    function balanceOf(address _owner) public pure returns (uint256) {
+        return 0;
+    }
+
+    function transfer(address _to, uint256 _value) public pure returns (bool success) {
+        return true;
+    }
+    
+    function transferFrom(address _from, address _to, uint256 _value) public pure returns (bool) {
+        return true;
+    }
+    
+    function approve(address _spender, uint256 _value) public pure returns (bool) {
+        return true;
+    }
+    
+    function allowance(address _owner, address _spender) public pure returns (uint256) {
+        return 0;
+    }
+
+}