add var-read-using-this detector

slither/detectors/all_detectors.py

@@ -3,6 +3,7 @@
 from .variables.uninitialized_state_variables import UninitializedStateVarsDetection
 from .variables.uninitialized_storage_variables import UninitializedStorageVars
 from .variables.uninitialized_local_variables import UninitializedLocalVars
+from .variables.var_read_using_this import VarReadUsingThis
 from .attributes.constant_pragma import ConstantPragma
 from .attributes.incorrect_solc import IncorrectSolc
 from .attributes.locked_ether import LockedEther

slither/detectors/variables/var_read_using_this.py

@@ -0,0 +1,54 @@
+from typing import List
+from slither.core.declarations import Function, SolidityVariable
+from slither.detectors.abstract_detector import AbstractDetector, DetectorClassification
+from slither.slithir.operations.high_level_call import HighLevelCall
+
+
+class VarReadUsingThis(AbstractDetector):
+    ARGUMENT = "var-read-using-this"
+    HELP = "Contract reads its own variable using `this`"
+    IMPACT = DetectorClassification.OPTIMIZATION
+    CONFIDENCE = DetectorClassification.MEDIUM
+
+    WIKI = "https://github.com/trailofbits/slither-private/wiki/Vulnerabilities-Description#var-read-using-this"
+
+    WIKI_TITLE = "Variable read using this"
+    WIKI_DESCRIPTION = "Contract reads its own variable using `this`, adding overhead of an unnecessary STATICCALL."
+    WIKI_EXPLOIT_SCENARIO = """
+```solidity
+contract C {
+    mapping(uint => address) public myMap;
+    function test(uint x) external returns(address) {
+        return this.myMap(x);
+    }
+}
+```
+"""
+
+    WIKI_RECOMMENDATION = "Read the variable directly from storage instead of calling the contract."
+
+    def _detect(self):
+        results = []
+        for c in self.contracts:
+            for func in c.functions:
+                for node in self._detect_var_read_using_this(func):
+                    info = [
+                        "The function ",
+                        func,
+                        " reads ",
+                        node,
+                        " with `this` which adds an extra STATICALL.\n",
+                    ]
+                    json = self.generate_result(info)
+                    results.append(json)
+
+        return results
+
+    def _detect_var_read_using_this(self, func: Function) -> List:
+        results = []
+        for node in func.nodes:
+            for ir in node.irs:
+                if isinstance(ir, HighLevelCall):
+                    if ir.destination == SolidityVariable("this") and ir.is_static_call():
+                        results.append(node)
+        return results

tests/detectors/var-read-using-this/0.4.25/var_read_using_this.sol

@@ -0,0 +1,33 @@
+
+contract VarReadUsingThis {
+    address public erc20;
+    mapping(uint => address) public myMap;
+    function bad1(uint x) external returns(address) {
+        return this.myMap(x);
+    }
+    function bad2() external returns(address) {
+        return this.erc20();
+    }
+    function bad3() external returns(address) {
+        if (this.erc20() == address(0)) revert();
+    }
+    function bad4() internal returns(address) {
+        for (uint x; x < 10; x++) {
+            address local = this.erc20();
+        } 
+    }
+    function good1(uint x) external returns(address) {
+        return myMap[x];
+    }
+    function good2() external returns(address) {
+        return erc20;
+    }
+    function good3() external returns(address) {
+        if (erc20 == address(0)) revert();
+    }
+    function good4() internal returns(address) {
+        for (uint x; x < 10; x++) {
+            address local = erc20;
+        } 
+    }
+}

tests/detectors/var-read-using-this/0.4.25/var_read_using_this.sol.0.4.25.VarReadUsingThis.json

@@ -0,0 +1,3 @@
+[
+    []
+]

tests/detectors/var-read-using-this/0.5.16/var_read_using_this.sol

@@ -0,0 +1,33 @@
+
+contract VarReadUsingThis {
+    address public erc20;
+    mapping(uint => address) public myMap;
+    function bad1(uint x) external returns(address) {
+        return this.myMap(x);
+    }
+    function bad2() external returns(address) {
+        return this.erc20();
+    }
+    function bad3() external returns(address) {
+        if (this.erc20() == address(0)) revert();
+    }
+    function bad4() internal returns(address) {
+        for (uint x; x < 10; x++) {
+            address local = this.erc20();
+        } 
+    }
+    function good1(uint x) external returns(address) {
+        return myMap[x];
+    }
+    function good2() external returns(address) {
+        return erc20;
+    }
+    function good3() external returns(address) {
+        if (erc20 == address(0)) revert();
+    }
+    function good4() internal returns(address) {
+        for (uint x; x < 10; x++) {
+            address local = erc20;
+        } 
+    }
+}