unit tests fixed & ballot_reentrant integration tests updated

remix-analyzer/src/solidity-analyzer/modules/checksEffectsInteraction.ts

@@ -4,7 +4,7 @@ import { isInteraction, isEffect, isLocalCallGraphRelevantNode, getFullQuallyfie
 import { default as algorithm } from './algorithmCategories'
 import { buildGlobalFuncCallGraph, resolveCallGraphSymbol, analyseCallGraph } from './functionCallGraph'
 import  AbstractAst from './abstractAstView'
-import { AnalyzerModule, ModuleAlgorithm, ModuleCategory, ReportObj, ContractHLAst, VariableDeclarationAstNode, FunctionHLAst, ContractCallGraph, Context} from './../../types'
+import { AnalyzerModule, ModuleAlgorithm, ModuleCategory, ReportObj, ContractHLAst, VariableDeclarationAstNode, FunctionHLAst, ContractCallGraph, Context, FunctionCallAstNode, AssignmentAstNode, UnaryOperationAstNode, InlineAssemblyAstNode} from './../../types'
 
 export default class checksEffectsInteraction implements AnalyzerModule {
   name: string = 'Check effects: '
@@ -14,7 +14,9 @@ export default class checksEffectsInteraction implements AnalyzerModule {
 
   abstractAst: AbstractAst = new AbstractAst()
 
-  visit: Function = this.abstractAst.build_visit((node: any) => isInteraction(node) || isEffect(node) || isLocalCallGraphRelevantNode(node))
+  visit: Function = this.abstractAst.build_visit((node: FunctionCallAstNode | AssignmentAstNode | UnaryOperationAstNode | InlineAssemblyAstNode) => (
+          node.nodeType === 'FunctionCall' && (isInteraction(node) || isLocalCallGraphRelevantNode(node))) || 
+          ((node.nodeType === 'Assignment' || node.nodeType === 'UnaryOperation' || node.nodeType === 'InlineAssembly') && isEffect(node)))
 
   report: Function = this.abstractAst.build_report(this._report.bind(this))
     
@@ -73,7 +75,7 @@ export default class checksEffectsInteraction implements AnalyzerModule {
     return isPotentialVulnerable
   }
 
-  private isLocalCallWithStateChange (node: any, context: Context): boolean {
+  private isLocalCallWithStateChange (node: FunctionCallAstNode, context: Context): boolean {
     if (isLocalCallGraphRelevantNode(node)) {
       const func = resolveCallGraphSymbol(context.callGraph, getFullQualifiedFunctionCallIdent(context.currentContract.node, node))
       return !func || (func && func.node['changesState'])

remix-analyzer/src/solidity-analyzer/modules/constantFunctions.ts

@@ -6,7 +6,7 @@ import { isLowLevelCall, isTransfer, isExternalDirectCall, isEffect, isLocalCall
 import { default as algorithm } from './algorithmCategories'
 import { buildGlobalFuncCallGraph, resolveCallGraphSymbol, analyseCallGraph } from './functionCallGraph'
 import  AbstractAst from './abstractAstView'
-import { AnalyzerModule, ModuleAlgorithm, ModuleCategory, ReportObj, ContractCallGraph, Context, ContractHLAst, FunctionHLAst, VariableDeclarationAstNode, FunctionCallGraph} from './../../types'
+import { AnalyzerModule, ModuleAlgorithm, ModuleCategory, ReportObj, ContractCallGraph, Context, ContractHLAst, FunctionHLAst, VariableDeclarationAstNode, FunctionCallGraph, FunctionCallAstNode} from './../../types'
 
 export default class constantFunctions implements AnalyzerModule {
   name: string = 'Constant functions: '
@@ -103,7 +103,7 @@ export default class constantFunctions implements AnalyzerModule {
           isDeleteUnaryOperation(node)
   }
 
-  private isCallOnNonConstExternalInterfaceFunction (node: any, context: Context): boolean {
+  private isCallOnNonConstExternalInterfaceFunction (node: FunctionCallAstNode, context: Context): boolean {
     if (isExternalDirectCall(node)) {
       const func: FunctionCallGraph | undefined = resolveCallGraphSymbol(context.callGraph, getFullQualifiedFunctionCallIdent(context.currentContract.node, node))
       return !func || (func && !isConstantFunction(func.node.node))

remix-analyzer/src/solidity-analyzer/modules/functionCallGraph.ts

 'use strict'
 
-import { FunctionHLAst, ContractHLAst, FunctionCallGraph, ContractCallGraph, Context } from "types"
+import { FunctionHLAst, ContractHLAst, FunctionCallGraph, ContractCallGraph, Context, FunctionCallAstNode } from "types"
 import { isLocalCallGraphRelevantNode,  isExternalDirectCall, getFullQualifiedFunctionCallIdent, getFullQuallyfiedFuncDefinitionIdent, getContractName } from './staticAnalysisCommon'
 
 function buildLocalFuncCallGraphInternal (functions: FunctionHLAst[], nodeFilter: any , extractNodeIdent: any, extractFuncDefIdent: Function): Record<string, FunctionCallGraph> {
@@ -42,9 +42,9 @@ function buildLocalFuncCallGraphInternal (functions: FunctionHLAst[], nodeFilter
  */
 export function buildGlobalFuncCallGraph (contracts: ContractHLAst[]): Record<string, ContractCallGraph> {
   const callGraph: Record<string, ContractCallGraph> = {}
-  contracts.forEach((contract) => {
-    const filterNodes: Function = (node) => { return isLocalCallGraphRelevantNode(node) || isExternalDirectCall(node) }
-    const getNodeIdent: Function = (node) => { return getFullQualifiedFunctionCallIdent(contract.node, node) }
+  contracts.forEach((contract: ContractHLAst) => {
+    const filterNodes: Function = (node: FunctionCallAstNode) => { return isLocalCallGraphRelevantNode(node) || isExternalDirectCall(node) }
+    const getNodeIdent: Function = (node: FunctionCallAstNode) => { return getFullQualifiedFunctionCallIdent(contract.node, node) }
     const getFunDefIdent: Function = (funcDef) => { return getFullQuallyfiedFuncDefinitionIdent(contract.node, funcDef.node, funcDef.parameters) }
 
     callGraph[getContractName(contract.node)] = { contract: contract, functions: buildLocalFuncCallGraphInternal(contract.functions, filterNodes, getNodeIdent, getFunDefIdent) }

remix-analyzer/src/solidity-analyzer/modules/similarVariableNames.ts

@@ -39,7 +39,7 @@ export default class similarVariableNames implements AnalyzerModule {
         this.findSimilarVarNames(vars).map((sim) => {
           warnings.push({
             warning: `${funcName} : Variables have very similar names ${sim.var1} and ${sim.var2}. ${hasModifiersComments} ${multipleContractsWithSameNameComments}`,
-            location: func['src']
+            location: func.node['src']
           })
         })
       })

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

@@ -168,12 +168,12 @@ function getFunctionCallType (func: FunctionCallAstNode): string {
  * @effectNode {ASTNode} Assignmnet node
  * @return {string} variable name written to
  */
-function getEffectedVariableName (effectNode: AssignmentAstNode | UnaryOperationAstNode) {
+function getEffectedVariableName (effectNode: AssignmentAstNode | UnaryOperationAstNode): string {
   if (!isEffect(effectNode)) throw new Error('staticAnalysisCommon.js: not an effect Node')
   if(effectNode.nodeType === 'Assignment' || effectNode.nodeType === 'UnaryOperation') {
     const IdentNode = findFirstSubNodeLTR(effectNode, exactMatch(nodeTypes.IDENTIFIER))
     return IdentNode.name
-  }
+  } else throw new Error('staticAnalysisCommon.js: wrong node type')
 }
 
 // developed keeping identifier node search in mind
@@ -237,7 +237,7 @@ function getSuperLocalCallName (superLocalCallNode: FunctionCallAstNode): string
  * @return {string} name of the contract the function is defined in
  */
 function getExternalDirectCallContractName (extDirectCall: FunctionCallAstNode): string {
-  if (!isExternalDirectCall(extDirectCall.expression)) throw new Error('staticAnalysisCommon.js: not an external direct call Node')
+  if (!isExternalDirectCall(extDirectCall)) throw new Error('staticAnalysisCommon.js: not an external direct call Node')
   return extDirectCall.expression.expression.typeDescriptions.typeString.replace(new RegExp(basicRegex.CONTRACTTYPE), '')
 }
 
@@ -265,7 +265,7 @@ function getThisLocalCallContractName (thisLocalCall: FunctionCallAstNode): stri
  * @return {string} name of the function called
  */
 function getExternalDirectCallMemberName (extDirectCall: FunctionCallAstNode): string {
-  if (!isExternalDirectCall(extDirectCall.expression)) throw new Error('staticAnalysisCommon.js: not an external direct call Node')
+  if (!isExternalDirectCall(extDirectCall)) throw new Error('staticAnalysisCommon.js: not an external direct call Node')
   return extDirectCall.expression.memberName
 }
 
@@ -436,7 +436,7 @@ function getFullQualifiedFunctionCallIdent (contract: ContractDefinitionAstNode,
   if (isLocalCall(func)) return getContractName(contract) + '.' + getLocalCallName(func) + '(' + getFunctionCallTypeParameterType(func) + ')'
   else if (isThisLocalCall(func.expression)) return getThisLocalCallContractName(func) + '.' + getThisLocalCallName(func) + '(' + getFunctionCallTypeParameterType(func) + ')'
   else if (isSuperLocalCall(func.expression)) return getContractName(contract) + '.' + getSuperLocalCallName(func) + '(' + getFunctionCallTypeParameterType(func) + ')'
-  else if (isExternalDirectCall(func.expression)) return getExternalDirectCallContractName(func) + '.' + getExternalDirectCallMemberName(func) + '(' + getFunctionCallTypeParameterType(func) + ')'
+  else if (isExternalDirectCall(func)) return getExternalDirectCallContractName(func) + '.' + getExternalDirectCallMemberName(func) + '(' + getFunctionCallTypeParameterType(func) + ')'
   else if (isLibraryCall(func.expression)) return getLibraryCallContractName(func.expression) + '.' + getLibraryCallMemberName(func) + '(' + getFunctionCallTypeParameterType(func) + ')'
   else throw new Error('staticAnalysisCommon.js: Can not get function name from non function call node')
 }
@@ -622,8 +622,8 @@ function isStorageVariableDeclaration (node: VariableDeclarationAstNode): boolea
  * @node {ASTNode} some AstNode
  * @return {bool}
  */
-function isInteraction (node: MemberAccessAstNode): boolean {
-  return isLLCall(node) || isLLSend(node) || isExternalDirectCall(node) || isTransfer(node)
+function isInteraction (node: FunctionCallAstNode): boolean {
+  return isLLCall(node.expression) || isLLSend(node.expression) || isExternalDirectCall(node) || isTransfer(node.expression)
 }
 
 /**
@@ -643,7 +643,7 @@ function isEffect (node: AssignmentAstNode | UnaryOperationAstNode | InlineAssem
  * @node {list Variable declaration} state variable declaration currently in scope
  * @return {bool}
  */
-function isWriteOnStateVariable (effectNode: AssignmentAstNode | InlineAssemblyAstNode | UnaryOperationAstNode, stateVariables: VariableDeclarationAstNode[]) {
+function isWriteOnStateVariable (effectNode: AssignmentAstNode | InlineAssemblyAstNode | UnaryOperationAstNode, stateVariables: VariableDeclarationAstNode[]): boolean {
   return effectNode.nodeType === "InlineAssembly" || (isEffect(effectNode) && isStateVariable(getEffectedVariableName(effectNode), stateVariables))
 }
 
@@ -796,8 +796,8 @@ function isLibraryCall (node: MemberAccessAstNode): boolean {
  * @node {ASTNode} some AstNode
  * @return {bool}
  */
-function isExternalDirectCall (node: MemberAccessAstNode): boolean {
-  return isMemberAccess(node, basicRegex.EXTERNALFUNCTIONTYPE, undefined, basicRegex.CONTRACTTYPE, undefined) && !isThisLocalCall(node) && !isSuperLocalCall(node)
+function isExternalDirectCall (node: FunctionCallAstNode): boolean {
+  return isMemberAccess(node.expression, basicRegex.EXTERNALFUNCTIONTYPE, undefined, basicRegex.CONTRACTTYPE, undefined) && !isThisLocalCall(node.expression) && !isSuperLocalCall(node.expression)
 }
 
 /**
@@ -908,7 +908,10 @@ function isLLSend (node: MemberAccessAstNode): boolean {
 function isLLCall (node: MemberAccessAstNode): boolean {
   return isMemberAccess(node,
           exactMatch(util.escapeRegExp(lowLevelCallTypes.CALL.type)),
-          undefined, exactMatch(basicTypes.ADDRESS), exactMatch(lowLevelCallTypes.CALL.ident))
+          undefined, exactMatch(basicTypes.ADDRESS), exactMatch(lowLevelCallTypes.CALL.ident)) ||
+          isMemberAccess(node,
+            exactMatch(util.escapeRegExp(lowLevelCallTypes.CALL.type)),
+            undefined, exactMatch(basicTypes.PAYABLE_ADDRESS), exactMatch(lowLevelCallTypes.CALL.ident))
 }
 
 /**

remix-analyzer/test/analysis/astBlocks/lowlevelCall.json

 {
     "sendAst": {
+      "argumentTypes": null,
+      "arguments":
+      [
+        {
+          "argumentTypes": null,
+          "hexValue": "31",
+          "id": 29,
+          "isConstant": false,
+          "isLValue": false,
+          "isPure": true,
+          "kind": "number",
+          "lValueRequested": false,
+          "nodeType": "Literal",
+          "src": "237:1:0",
+          "subdenomination": null,
+          "typeDescriptions":
+          {
+            "typeIdentifier": "t_rational_1_by_1",
+            "typeString": "int_const 1"
+          },
+          "value": "1"
+        }
+      ],
+      "expression":
+      {
         "argumentTypes":
         [
           {
@@ -37,24 +62,71 @@
           "typeString": "function (uint256) returns (bool)"
         }
       },
+      "id": 30,
+      "isConstant": false,
+      "isLValue": false,
+      "isPure": false,
+      "kind": "functionCall",
+      "lValueRequested": false,
+      "names": [],
+      "nodeType": "FunctionCall",
+      "src": "227:12:0",
+      "tryCall": false,
+      "typeDescriptions":
+      {
+        "typeIdentifier": "t_bool",
+        "typeString": "bool"
+      }
+    },
     "callAst": {
-        "argumentTypes": null,
+      "argumentTypes": null,
+      "arguments":
+      [
+        {
+          "argumentTypes": null,
+          "hexValue": "",
+          "id": 15,
+          "isConstant": false,
+          "isLValue": false,
+          "isPure": true,
+          "kind": "string",
+          "lValueRequested": false,
+          "nodeType": "Literal",
+          "src": "142:2:0",
+          "subdenomination": null,
+          "typeDescriptions":
+          {
+            "typeIdentifier": "t_stringliteral_c5d2460186f7233c927e7db2dcc703c0e500b653ca82273b7bfad8045d85a470",
+            "typeString": "literal_string \"\""
+          },
+          "value": ""
+        }
+      ],
+      "expression":
+      {
+        "argumentTypes":
+        [
+          {
+            "typeIdentifier": "t_stringliteral_c5d2460186f7233c927e7db2dcc703c0e500b653ca82273b7bfad8045d85a470",
+            "typeString": "literal_string \"\""
+          }
+        ],
         "expression":
         {
           "argumentTypes": null,
-          "id": 9,
-          "name": "a",
+          "id": 13,
+          "name": "addr",
           "nodeType": "Identifier",
           "overloadedDeclarations": [],
-          "referencedDeclaration": 6,
-          "src": "91:1:0",
+          "referencedDeclaration": 4,
+          "src": "132:4:0",
           "typeDescriptions":
           {
-            "typeIdentifier": "t_address",
-            "typeString": "address"
+            "typeIdentifier": "t_address_payable",
+            "typeString": "address payable"
           }
         },
-        "id": 12,
+        "id": 14,
         "isConstant": false,
         "isLValue": false,
         "isPure": false,
@@ -62,13 +134,29 @@
         "memberName": "call",
         "nodeType": "MemberAccess",
         "referencedDeclaration": null,
-        "src": "91:6:0",
+        "src": "132:9:0",
         "typeDescriptions":
         {
           "typeIdentifier": "t_function_barecall_payable$_t_bytes_memory_ptr_$returns$_t_bool_$_t_bytes_memory_ptr_$",
           "typeString": "function (bytes memory) payable returns (bool,bytes memory)"
         }
       },
+      "id": 16,
+      "isConstant": false,
+      "isLValue": false,
+      "isPure": false,
+      "kind": "functionCall",
+      "lValueRequested": false,
+      "names": [],
+      "nodeType": "FunctionCall",
+      "src": "132:13:0",
+      "tryCall": false,
+      "typeDescriptions":
+      {
+        "typeIdentifier": "t_tuple$_t_bool_$_t_bytes_memory_ptr_$",
+        "typeString": "tuple(bool,bytes memory)"
+      }
+    },
    "callcodeAst": {
         "argumentTypes": [],
         "expression":

remix-analyzer/test/analysis/astBlocks/storageVariableNodes.json

@@ -7,7 +7,7 @@
     "overrides": null,
     "scope": 33,
     "src": "174:11:0",
-    "stateVariable": false,
+    "stateVariable": true,
     "storageLocation": "storage",
     "typeDescriptions":
     {

remix-analyzer/test/analysis/staticAnalysisCommon-test.ts

@@ -278,11 +278,10 @@ test('staticAnalysisCommon.isStorageVariableDeclaration', function (t) {
 })
 
 test('staticAnalysisCommon.isInteraction', function (t) {
-  t.plan(6)
+  t.plan(5)
   t.ok(common.isInteraction(lowlevelCall.sendAst), 'send is interaction')
   t.ok(common.isInteraction(lowlevelCall.callAst), 'call is interaction')
-  t.ok(common.isInteraction(externalDirect.expression), 'ExternalDirecCall is interaction')
-  t.notOk(common.isInteraction(lowlevelCall.callcodeAst), 'callcode is not interaction')
+  t.ok(common.isInteraction(externalDirect), 'ExternalDirectCall is interaction')
   t.notOk(common.isInteraction(lowlevelCall.delegatecallAst), 'delegatecall is not interaction')
   t.notOk(common.isInteraction(localCall), 'local call is not interaction')
 })
@@ -364,7 +363,7 @@ test('staticAnalysisCommon.isExternalDirectCall', function (t) {
   t.notOk(common.isThisLocalCall(externalDirect), 'is this.local_method() used should not work')
   t.notOk(common.isBlockTimestampAccess(externalDirect), 'is block.timestamp used should not work')
   t.notOk(common.isNowAccess(externalDirect), 'is now used should not work')
-  t.ok(common.isExternalDirectCall(externalDirect.expression), 'c.f() should be external direct call')
+  t.ok(common.isExternalDirectCall(externalDirect), 'c.f() should be external direct call')
   t.notOk(common.isExternalDirectCall(thisLocalCall.expression), 'this local call is not an exernal call')
 })
 
@@ -414,8 +413,8 @@ test('staticAnalysisCommon.isLocalCall', function (t) {
 
 test('staticAnalysisCommon.isLowLevelCall', function (t) {
   t.plan(3)
-  t.ok(common.isLLSend(lowlevelCall.sendAst) && common.isLowLevelCall(lowlevelCall.sendAst), 'send is llc should work')
-  t.ok(common.isLLCall(lowlevelCall.callAst) && common.isLowLevelCall(lowlevelCall.callAst), 'call is llc should work')
+  t.ok(common.isLLSend(lowlevelCall.sendAst.expression) && common.isLowLevelCall(lowlevelCall.sendAst.expression), 'send is llc should work')
+  t.ok(common.isLLCall(lowlevelCall.callAst.expression) && common.isLowLevelCall(lowlevelCall.callAst.expression), 'call is llc should work')
   t.ok(common.isLLDelegatecall(lowlevelCall.delegatecallAst) && common.isLowLevelCall(lowlevelCall.delegatecallAst), 'delegatecall is llc should work')
 })