Implement reentrancy error validation rule #7

celo-org · Oct 17, 2017 · cd97fd7 · cd97fd7
1 parent d44af05
commit cd97fd7
Showing 1 changed file with 39 additions and 21 deletions.
diff --git a/lib/rules/security/reentrancy.js b/lib/rules/security/reentrancy.js
@@ -30,8 +30,9 @@ class ReentrancyChecker extends BaseChecker {
 
     _checkAssignment(ctx) {
         const effects = Effects.of(ctx);
+        const assignOperator = AssignOperator.of(ctx);
 
-        if (isAssignOperator(ctx) && effects && !effects.isAllowedAssign(ctx)) {
+        if (assignOperator && effects && effects.isNotAllowed(assignOperator)) {
             this._warn(ctx);
         }
     }
@@ -48,6 +49,27 @@ class ReentrancyChecker extends BaseChecker {
 }
 
 
+class Effects {
+
+    static of (ctx) {
+        return findPropertyInParents(ctx, 'effects');
+    }
+
+    constructor (statesScope) {
+        this.states = statesScope && statesScope.states;
+        this.hasTransfer = false;
+    }
+
+    isNotAllowed(operator) {
+        return this.hasTransfer && operator.modifyOneOf(this.states);
+    }
+
+    trackTransfer() {
+        this.hasTransfer = true;
+    }
+}
+
+
 class StateDeclarationScope {
 
     static of (ctx) {
@@ -100,6 +122,7 @@ class ContractPart {
     }
 }
 
+
 class StateDefinition {
 
     constructor(ctx) {
@@ -114,35 +137,30 @@ class StateDefinition {
 }
 
 
-class Effects {
+class AssignOperator {
 
-    static of (ctx) {
-        return findPropertyInParents(ctx, 'effects');
-    }
+    static of(ctx) {
+        const hasThreeItems = _.size(ctx.children) === 3;
+        const hasAssignOperator = ctx.children[1] && ctx.children[1].getText() === '=';
 
-    constructor (statesScope) {
-        this.states = statesScope && statesScope.states;
-        this.hasTransfer = false;
+        if (hasThreeItems && hasAssignOperator) {
+            return new AssignOperator(ctx);
+        }
     }
 
-    isAllowedAssign(ctx) {
-        const assignee = ctx.children[0].getText();
-
-        return !(this.hasTransfer && this._isContainsStateName(assignee));
+    constructor(ctx) {
+        this.ctx = ctx;
     }
 
-    trackTransfer() {
-        this.hasTransfer = true;
-    }
+    modifyOneOf (states) {
+        const assigneeText = this._assignee().getText();
 
-    _isContainsStateName(expressionText) {
-        return this.states.some(i => expressionText.includes(i));
+        return states.some(curStateName => assigneeText.includes(curStateName));
     }
-}
 
-
-function isAssignOperator(ctx) {
-    return _.size(ctx.children) === 3 && ctx.children[1].getText() === '=';
+    _assignee() {
+        return this.ctx.children[0];
+    }
 }