Static Analysis: ERC20 decimals type check

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

+var name = 'ERC20: '
+var desc = 'Decimal should be uint8'
+var categories = require('./categories')
+var common = require('./staticAnalysisCommon')
+var AbstractAst = require('./abstractAstView')
+
+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://github.com/ethereum/EIPs/blob/master/EIPS/eip-20.md'
+        })
+      }
+    }
+  })
+
+  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,
+  Module: erc20Decimals
+}

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

 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/staticAnalysisCommon.js

@@ -266,6 +266,18 @@ function getDeclaredVariableName (varDeclNode) {
 }
 
 /**
+ * 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:
  * contract foo {
@@ -757,7 +769,7 @@ function isBlockTimestampAccess (node) {
  * @return {bool}
  */
 function isBlockBlockHashAccess (node) {
-  return isSpecialVariableAccess(node, specialVariables.BLOCKHASH)
+  return isSpecialVariableAccess(node, specialVariables.BLOCKHASH) || isBuiltinFunctionCall(node) && getLocalCallName(node) === 'blockhash'
 }
 
 /**
@@ -919,6 +931,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
@@ -948,6 +964,7 @@ module.exports = {
   getContractName: getContractName,
   getEffectedVariableName: getEffectedVariableName,
   getDeclaredVariableName: getDeclaredVariableName,
+  getDeclaredVariableType: getDeclaredVariableType,
   getLocalCallName: getLocalCallName,
   getInheritsFromName: getInheritsFromName,
   getExternalDirectCallContractName: getExternalDirectCallContractName,
@@ -1033,6 +1050,7 @@ module.exports = {
     nodeType: nodeType,
     name: name,
     operator: operator,
-    buildFunctionSignature: buildFunctionSignature
+    buildFunctionSignature: buildFunctionSignature,
+    buildAbiSignature: buildAbiSignature
   }
 }

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

@@ -30,7 +30,8 @@ var testFiles = [
   'selfdestruct.sol',
   'deleteDynamicArray.sol',
   'blockLevelCompare.sol',
-  'intDivisionTruncate.sol'
+  'intDivisionTruncate.sol',
+  'ERC20.sol'
 ]
 
 var testFileAsts = {}
@@ -66,7 +67,8 @@ test('Integration test thisLocal.js', function (t) {
     'selfdestruct.sol': 0,
     'deleteDynamicArray.sol': 0,
     'blockLevelCompare.sol': 0,
-    'intDivisionTruncate.sol': 0
+    'intDivisionTruncate.sol': 0,
+    'ERC20.sol': 0
   }
 
   runModuleOnFiles(module, t, (file, report) => {
@@ -100,7 +102,8 @@ test('Integration test checksEffectsInteraction.js', function (t) {
     'selfdestruct.sol': 0,
     'deleteDynamicArray.sol': 0,
     'blockLevelCompare.sol': 0,
-    'intDivisionTruncate.sol': 0
+    'intDivisionTruncate.sol': 0,
+    'ERC20.sol': 0
   }
 
   runModuleOnFiles(module, t, (file, report) => {
@@ -134,7 +137,8 @@ test('Integration test constantFunctions.js', function (t) {
     'selfdestruct.sol': 1,
     'deleteDynamicArray.sol': 0,
     'blockLevelCompare.sol': 0,
-    'intDivisionTruncate.sol': 0
+    'intDivisionTruncate.sol': 0,
+    'ERC20.sol': 0
   }
 
   runModuleOnFiles(module, t, (file, report) => {
@@ -168,7 +172,8 @@ test('Integration test inlineAssembly.js', function (t) {
     'selfdestruct.sol': 0,
     'deleteDynamicArray.sol': 0,
     'blockLevelCompare.sol': 0,
-    'intDivisionTruncate.sol': 0
+    'intDivisionTruncate.sol': 0,
+    'ERC20.sol': 0
   }
 
   runModuleOnFiles(module, t, (file, report) => {
@@ -202,7 +207,8 @@ test('Integration test txOrigin.js', function (t) {
     'selfdestruct.sol': 0,
     'deleteDynamicArray.sol': 0,
     'blockLevelCompare.sol': 0,
-    'intDivisionTruncate.sol': 0
+    'intDivisionTruncate.sol': 0,
+    'ERC20.sol': 0
   }
 
   runModuleOnFiles(module, t, (file, report) => {
@@ -236,7 +242,8 @@ test('Integration test gasCosts.js', function (t) {
     'selfdestruct.sol': 0,
     'deleteDynamicArray.sol': 2,
     'blockLevelCompare.sol': 1,
-    'intDivisionTruncate.sol': 1
+    'intDivisionTruncate.sol': 1,
+    'ERC20.sol': 2
   }
 
   runModuleOnFiles(module, t, (file, report) => {
@@ -270,7 +277,8 @@ test('Integration test similarVariableNames.js', function (t) {
     'selfdestruct.sol': 0,
     'deleteDynamicArray.sol': 1,
     'blockLevelCompare.sol': 0,
-    'intDivisionTruncate.sol': 0
+    'intDivisionTruncate.sol': 0,
+    'ERC20.sol': 0
   }
 
   runModuleOnFiles(module, t, (file, report) => {
@@ -304,7 +312,8 @@ test('Integration test inlineAssembly.js', function (t) {
     'selfdestruct.sol': 0,
     'deleteDynamicArray.sol': 0,
     'blockLevelCompare.sol': 0,
-    'intDivisionTruncate.sol': 0
+    'intDivisionTruncate.sol': 0,
+    'ERC20.sol': 0
   }
 
   runModuleOnFiles(module, t, (file, report) => {
@@ -338,7 +347,8 @@ test('Integration test blockTimestamp.js', function (t) {
     'selfdestruct.sol': 0,
     'deleteDynamicArray.sol': 0,
     'blockLevelCompare.sol': 0,
-    'intDivisionTruncate.sol': 0
+    'intDivisionTruncate.sol': 0,
+    'ERC20.sol': 0
   }
 
   runModuleOnFiles(module, t, (file, report) => {
@@ -372,7 +382,8 @@ test('Integration test lowLevelCalls.js', function (t) {
     'selfdestruct.sol': 0,
     'deleteDynamicArray.sol': 0,
     'blockLevelCompare.sol': 0,
-    'intDivisionTruncate.sol': 0
+    'intDivisionTruncate.sol': 0,
+    'ERC20.sol': 0
   }
 
   runModuleOnFiles(module, t, (file, report) => {
@@ -406,7 +417,8 @@ test('Integration test blockBlockhash.js', function (t) {
     'selfdestruct.sol': 0,
     'deleteDynamicArray.sol': 0,
     'blockLevelCompare.sol': 0,
-    'intDivisionTruncate.sol': 0
+    'intDivisionTruncate.sol': 0,
+    'ERC20.sol': 0
   }
 
   runModuleOnFiles(module, t, (file, report) => {
@@ -440,7 +452,8 @@ test('Integration test noReturn.js', function (t) {
     'selfdestruct.sol': 0,
     'deleteDynamicArray.sol': 0,
     'blockLevelCompare.sol': 0,
-    'intDivisionTruncate.sol': 0
+    'intDivisionTruncate.sol': 0,
+    'ERC20.sol': 0
   }
 
   runModuleOnFiles(module, t, (file, report) => {
@@ -474,7 +487,8 @@ test('Integration test selfdestruct.js', function (t) {
     'selfdestruct.sol': 3,
     'deleteDynamicArray.sol': 0,
     'blockLevelCompare.sol': 0,
-    'intDivisionTruncate.sol': 5
+    'intDivisionTruncate.sol': 5,
+    'ERC20.sol': 0
   }
 
   runModuleOnFiles(module, t, (file, report) => {
@@ -508,7 +522,8 @@ test('Integration test guardConditions.js', function (t) {
     'selfdestruct.sol': 0,
     'deleteDynamicArray.sol': 1,
     'blockLevelCompare.sol': 0,
-    'intDivisionTruncate.sol': 1
+    'intDivisionTruncate.sol': 1,
+    'ERC20.sol': 0
   }
 
   runModuleOnFiles(module, t, (file, report) => {
@@ -542,7 +557,8 @@ test('Integration test deleteDynamicArrays.js', function (t) {
     'selfdestruct.sol': 0,
     'deleteDynamicArray.sol': 2,
     'blockLevelCompare.sol': 0,
-    'intDivisionTruncate.sol': 0
+    'intDivisionTruncate.sol': 0,
+    'ERC20.sol': 0
   }
 
   runModuleOnFiles(module, t, (file, report) => {
@@ -576,7 +592,8 @@ test('Integration test assignAndCompare.js', function (t) {
     'selfdestruct.sol': 0,
     'deleteDynamicArray.sol': 0,
     'blockLevelCompare.sol': 8,
-    'intDivisionTruncate.sol': 0
+    'intDivisionTruncate.sol': 0,
+    'ERC20.sol': 0
   }
 
   runModuleOnFiles(module, t, (file, report) => {
@@ -610,7 +627,8 @@ test('Integration test intDivisionTruncate.js', function (t) {
     'selfdestruct.sol': 0,
     'deleteDynamicArray.sol': 0,
     'blockLevelCompare.sol': 0,
-    'intDivisionTruncate.sol': 2
+    'intDivisionTruncate.sol': 2,
+    'ERC20.sol': 0
   }
 
   runModuleOnFiles(module, t, (file, report) => {
@@ -618,6 +636,41 @@ test('Integration test intDivisionTruncate.js', function (t) {
   })
 })
 
+test('Integration test erc20Decimal.js', function (t) {
+  t.plan(testFiles.length)
+
+  var module = require('../../src/analysis/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
+  }
+
+  runModuleOnFiles(module, t, (file, report) => {
+    t.equal(report.length, lengthCheck[file], `${file} has right amount of erc20Decimals warnings`)
+  })
+})
+
 // #################### Helpers
 function runModuleOnFiles (module, t, cb) {
   var statRunner = new StatRunner()

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

+
+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;
+    }
+
+}
\ No newline at end of file