Update rule: reentrancy

lib/common/tree-traversing.js

@@ -74,14 +74,14 @@ TreeTraversing.hasMethodCalls = function hasMethodCalls(node, methodNames) {
   return methodNames.includes(text)
 }
 
-TreeTraversing.findPropertyInParents = function findPropertyInParents(ctx, property) {
-  let curCtx = ctx
+TreeTraversing.findPropertyInParents = function findPropertyInParents(node, property) {
+  let curNode = node
 
-  while (curCtx !== null && !curCtx[property]) {
-    curCtx = curCtx.parentCtx
+  while (curNode !== undefined && !curNode[property]) {
+    curNode = curNode.parent
   }
 
-  return curCtx && curCtx[property]
+  return curNode && curNode[property]
 }
 
 module.exports = TreeTraversing

lib/index.js

@@ -10,7 +10,7 @@ const checkers = require('./rules/index')
 function parseInput(inputStr) {
   try {
     // first we try to parse the string as we normally do
-    return parser.parse(inputStr, { loc: true })
+    return parser.parse(inputStr, { loc: true, range: true })
   } catch (e) {
     try {
       // using 'loc' may throw when inputStr is empty or only has comments

lib/rules/index.js

@@ -45,7 +45,7 @@ function coreRules(meta) {
     ...miscellaneous(reporter, config, tokens),
     ...naming(reporter, config),
     ...order(reporter, tokens),
-    ...security(reporter, config)
+    ...security(reporter, config, inputSrc)
   ]
 }
 

lib/rules/security/index.js

@@ -16,7 +16,7 @@ const NotRelyOnTimeChecker = require('./not-rely-on-time')
 const ReentrancyChecker = require('./reentrancy')
 const StateVisibilityChecker = require('./state-visibility')
 
-module.exports = function security(reporter, config) {
+module.exports = function security(reporter, config, inputSrc) {
   return [
     new AvoidCallValueChecker(reporter),
     new AvoidLowLevelCallsChecker(reporter),
@@ -33,7 +33,7 @@ module.exports = function security(reporter, config) {
     new NoInlineAssemblyChecker(reporter),
     new NotRelyOnBlockHashChecker(reporter),
     new NotRelyOnTimeChecker(reporter),
-    new ReentrancyChecker(reporter),
+    new ReentrancyChecker(reporter, inputSrc),
     new StateVisibilityChecker(reporter)
   ]
 }

lib/rules/security/reentrancy.js

@@ -2,7 +2,7 @@ const _ = require('lodash')
 const BaseChecker = require('./../base-checker')
 const TreeTraversing = require('./../../common/tree-traversing')
 
-const { typeOf, hasMethodCalls, findPropertyInParents } = TreeTraversing
+const { hasMethodCalls, findPropertyInParents } = TreeTraversing
 
 const ruleId = 'reentrancy'
 const meta = {
@@ -47,38 +47,42 @@ const meta = {
 }
 
 class ReentrancyChecker extends BaseChecker {
-  constructor(reporter) {
+  constructor(reporter, inputSrc) {
     super(reporter, ruleId, meta)
+    this.inputSrc = inputSrc
   }
 
-  enterContractDefinition(ctx) {
-    ctx.stateDeclarationScope = new StateDeclarationScope()
-    const scope = ctx.stateDeclarationScope
+  ContractDefinition(node) {
+    node.stateDeclarationScope = new StateDeclarationScope()
+    const scope = node.stateDeclarationScope
 
-    new ContractDefinition(ctx).stateDefinitions().forEach(i => scope.trackStateDeclaration(i))
+    new ContractDefinition(node).stateDefinitions().forEach(i => scope.trackStateDeclaration(i))
   }
 
-  enterFunctionDefinition(ctx) {
-    ctx.effects = new Effects(StateDeclarationScope.of(ctx))
+  FunctionDefinition(node) {
+    node.effects = new Effects(StateDeclarationScope.of(node))
   }
 
-  enterExpression(ctx) {
-    this._checkAssignment(ctx)
-    this._checkSendCall(ctx)
+  ExpressionStatement(node) {
+    this._checkAssignment(node)
   }
 
-  _checkAssignment(ctx) {
-    const effects = Effects.of(ctx)
-    const assignOperator = AssignOperator.of(ctx)
+  MemberAccess(node) {
+    this._checkSendCall(node)
+  }
+
+  _checkAssignment(node) {
+    const assignOperator = AssignOperator.of(node, this.inputSrc)
+    const effects = Effects.of(node)
 
     if (assignOperator && effects && effects.isNotAllowed(assignOperator)) {
-      this._warn(ctx)
+      this._warn(node)
     }
   }
 
-  _checkSendCall(ctx) {
-    if (hasMethodCalls(ctx, ['send', 'transfer'])) {
-      Effects.of(ctx).trackTransfer()
+  _checkSendCall(node) {
+    if (hasMethodCalls(node, ['send', 'transfer'])) {
+      Effects.of(node).trackTransfer()
     }
   }
 
@@ -88,8 +92,8 @@ class ReentrancyChecker extends BaseChecker {
 }
 
 class Effects {
-  static of(ctx) {
-    return findPropertyInParents(ctx, 'effects')
+  static of(node) {
+    return findPropertyInParents(node, 'effects')
   }
 
   constructor(statesScope) {
@@ -107,8 +111,8 @@ class Effects {
 }
 
 class StateDeclarationScope {
-  static of(ctx) {
-    return findPropertyInParents(ctx, 'stateDeclarationScope')
+  static of(node) {
+    return findPropertyInParents(node, 'stateDeclarationScope')
   }
 
   constructor() {
@@ -122,70 +126,69 @@ class StateDeclarationScope {
 }
 
 class ContractDefinition {
-  constructor(ctx) {
-    this.ctx = ctx
+  constructor(node) {
+    this.node = node
   }
 
   stateDefinitions() {
-    return this.ctx.children
+    return this.node.subNodes
       .map(i => new ContractPart(i))
       .filter(i => i.isStateDefinition())
       .map(i => i.getStateDefinition())
   }
 }
 
 class ContractPart {
-  constructor(ctx) {
-    this.ctx = ctx
+  constructor(node) {
+    this.node = node
   }
 
   isStateDefinition() {
-    return typeOf(this._firstChild()) === 'stateVariableDeclaration'
+    return this.node.type === 'StateVariableDeclaration'
   }
 
   getStateDefinition() {
     return new StateDefinition(this._firstChild())
   }
 
   _firstChild() {
-    return _.first(this.ctx.children)
+    return _.first(this.node.variables)
   }
 }
 
 class StateDefinition {
-  constructor(ctx) {
-    this.ctx = ctx
+  constructor(node) {
+    this.node = node
   }
 
   stateName() {
-    return _(this.ctx.children)
-      .find(i => typeOf(i) === 'identifier')
-      .getText()
+    return this.node.name
   }
 }
 
 class AssignOperator {
-  static of(ctx) {
-    const hasThreeItems = _.size(ctx.children) === 3
-    const hasAssignOperator = ctx.children[1] && ctx.children[1].getText() === '='
-
-    if (hasThreeItems && hasAssignOperator) {
-      return new AssignOperator(ctx)
+  static of(node, inputSrc) {
+    if (node.expression.type === 'BinaryOperation') {
+      return new AssignOperator(node, inputSrc)
     }
   }
 
-  constructor(ctx) {
-    this.ctx = ctx
+  constructor(node, inputSrc) {
+    this.node = node
+    this.inputSrc = inputSrc
   }
 
   modifyOneOf(states) {
-    const assigneeText = this._assignee().getText()
+    const assigneeText = this._assignee()
 
     return states.some(curStateName => assigneeText.includes(curStateName))
   }
 
   _assignee() {
-    return this.ctx.children[0]
+    return this.inputSrc.slice(
+      this.node.expression.left.range[0],
+      this.node.expression.left.range[1] + 1
+    )
   }
 }