ERC20/721 detector: move is_possible_* function to contract.has_an_erc*_function

Add contract.is_erc721
6 years ago · 5e1eb42311
parent c7509c7e5e
commit 5e1eb42311
7 changed files with 271 additions and 41 deletions
--- a/scripts/tests_generate_expected_json_4.sh
+++ b/scripts/tests_generate_expected_json_4.sh
@ -23,6 +23,7 @@ generate_expected_json(){
 #generate_expected_json tests/deprecated_calls.sol "deprecated-standards"
 #generate_expected_json tests/erc20_indexed.sol "erc20-indexed"
 #generate_expected_json tests/incorrect_erc20_interface.sol "erc20-interface"
+#generate_expected_json tests/incorrect_erc721_interface.sol "erc721-interface"
 #generate_expected_json tests/uninitialized.sol "uninitialized-state"
 #generate_expected_json tests/backdoor.sol "backdoor"
 #generate_expected_json tests/backdoor.sol "suicidal"
--- a/scripts/travis_test_4.sh
+++ b/scripts/travis_test_4.sh
@ -72,6 +72,7 @@ test_slither(){
 test_slither tests/deprecated_calls.sol "deprecated-standards"
 test_slither tests/erc20_indexed.sol "erc20-indexed"
 test_slither tests/incorrect_erc20_interface.sol "erc20-interface"
+test_slither tests/incorrect_erc721_interface.sol "erc721-interface"
 test_slither tests/uninitialized.sol "uninitialized-state"
 test_slither tests/backdoor.sol "backdoor"
 test_slither tests/backdoor.sol "suicidal"
--- a/slither/core/declarations/contract.py
+++ b/slither/core/declarations/contract.py
@ -527,6 +527,14 @@ class Contract(ChildSlither, SourceMapping):
        """
        return all((not f.is_implemented) for f in self.functions)

+    # endregion
+    ###################################################################################
+    ###################################################################################
+    # region ERC conformance
+    ###################################################################################
+    ###################################################################################
+
+
    def is_erc20(self):
        """
            Check if the contract is an erc20 token
@ -540,6 +548,44 @@ class Contract(ChildSlither, SourceMapping):
               'transferFrom(address,address,uint256)' in full_names and\
               'approve(address,uint256)' in full_names

+    def is_erc721(self):
+        full_names = set([f.full_name for f in self.functions])
+        return self.is_erc20() and\
+               'ownerOf(uint256)' in full_names and\
+               'safeTransferFrom(address,address,uint256,bytes)' in full_names and\
+               'safeTransferFrom(address,address,uint256)' in full_names and\
+               'setApprovalForAll(address,bool)' in full_names and\
+               'getApproved(uint256)' in full_names and\
+               'isApprovedForAll(address,address)' in full_names
+
+    def has_an_erc20_function(self):
+        """
+        Checks if the provided contract could be attempting to implement ERC20 standards.
+        :param contract: The contract to check for token compatibility.
+        :return: Returns a boolean indicating if the provided contract met the token standard.
+        """
+        full_names = set([f.full_name for f in self.functions])
+        return 'transfer(address,uint256)' in full_names or \
+               'transferFrom(address,address,uint256)' in full_names or \
+               'approve(address,uint256)' in full_names
+
+    def has_an_erc721_function(self):
+        """
+        Checks if the provided contract could be attempting to implement ERC721 standards.
+        :param contract: The contract to check for token compatibility.
+        :return: Returns a boolean indicating if the provided contract met the token standard.
+        """
+        full_names = set([f.full_name for f in self.functions])
+        return self.has_an_erc20_function() and \
+               ('ownerOf(uint256)' in full_names or
+                'safeTransferFrom(address,address,uint256,bytes)' in full_names or
+                'safeTransferFrom(address,address,uint256)' in full_names or
+                'setApprovalForAll(address,bool)' in full_names or
+                'getApproved(uint256)' in full_names or
+                'isApprovedForAll(address,address)' in full_names)
+
+
+
    # endregion
    ###################################################################################
    ###################################################################################
--- a/slither/detectors/erc/incorrect_erc20_interface.py
+++ b/slither/detectors/erc/incorrect_erc20_interface.py
@ -5,18 +5,6 @@ Some contracts do not return a bool on transfer/transferFrom/approve, which may
 from slither.detectors.abstract_detector import AbstractDetector, DetectorClassification


-def is_possible_erc20(contract):
-    """
-    Checks if the provided contract could be attempting to implement ERC20 standards.
-    :param contract: The contract to check for token compatibility.
-    :return: Returns a boolean indicating if the provided contract met the token standard.
-    """
-    full_names = set([f.full_name for f in contract.functions])
-    return 'transfer(address,uint256)' in full_names or \
-           'transferFrom(address,address,uint256)' in full_names or \
-           'approve(address,uint256)' in full_names
-
-
 class IncorrectERC20InterfaceDetection(AbstractDetector):
    """
    Incorrect ERC20 Interface
@ -74,14 +62,12 @@ contract Token{
            list(str) : list of incorrect function signatures
        """
        # Verify this is an ERC20 contract.
-        if not is_possible_erc20(contract):
+        if not contract.has_an_erc20_function():
            return []

        # If this contract implements a function from ERC721, we can assume it is an ERC721 token. These tokens
        # offer functions which are similar to ERC20, but are not compatible.
-        # NOTE: This detector is dependent on this one, so we import here to avoid errors.
-        from slither.detectors.erc.incorrect_erc721_interface import is_possible_erc721
-        if is_possible_erc721(contract):
+        if contract.has_an_erc721_function():
            return []

        functions = [f for f in contract.functions if IncorrectERC20InterfaceDetection.incorrect_erc20_interface(f.signature)]
--- a/slither/detectors/erc/incorrect_erc721_interface.py
+++ b/slither/detectors/erc/incorrect_erc721_interface.py
@ -2,23 +2,6 @@
 Detect incorrect erc721 interface.
 """
 from slither.detectors.abstract_detector import AbstractDetector, DetectorClassification
-from slither.detectors.erc.incorrect_erc20_interface import is_possible_erc20
-
-
-def is_possible_erc721(contract):
-    """
-    Checks if the provided contract could be attempting to implement ERC721 standards.
-    :param contract: The contract to check for token compatibility.
-    :return: Returns a boolean indicating if the provided contract met the token standard.
-    """
-    full_names = set([f.full_name for f in contract.functions])
-    return is_possible_erc20(contract) and \
-        ('ownerOf(uint256)' in full_names or
-            'safeTransferFrom(address,address,uint256,bytes)' in full_names or
-            'safeTransferFrom(address,address,uint256)' in full_names or
-            'setApprovalForAll(address,bool)' in full_names or
-            'getApproved(uint256)' in full_names or
-            'isApprovedForAll(address,address)' in full_names)


 class IncorrectERC721InterfaceDetection(AbstractDetector):
@ -85,7 +68,7 @@ contract Token{
        """

        # Verify this is an ERC721 contract.
-        if not is_possible_erc721(contract) or not is_possible_erc20(contract):
+        if not contract.has_an_erc721_function() or not contract.has_an_erc20_function():
            return []

        functions = [f for f in contract.functions if IncorrectERC721InterfaceDetection.incorrect_erc721_interface(f.signature)]
--- a/tests/expected_json/incorrect_erc20_interface.erc20-interface.json
+++ b/tests/expected_json/incorrect_erc20_interface.erc20-interface.json
@ -3,20 +3,184 @@
    "check": "erc20-interface",
    "impact": "Medium",
    "confidence": "High",
-    "description": "Token (tests/incorrect_erc20_interface.sol#3-7) has incorrect ERC20 function interface(s):\n\t-transfer (tests/incorrect_erc20_interface.sol#5)\n",
+    "description": "Token (tests/incorrect_erc20_interface.sol#3-10) has incorrect ERC20 function interface(s):\n\t-transfer (tests/incorrect_erc20_interface.sol#4)\n\t-approve (tests/incorrect_erc20_interface.sol#5)\n\t-transferFrom (tests/incorrect_erc20_interface.sol#6)\n\t-totalSupply (tests/incorrect_erc20_interface.sol#7)\n\t-balanceOf (tests/incorrect_erc20_interface.sol#8)\n\t-allowance (tests/incorrect_erc20_interface.sol#9)\n",
    "elements": [
+      {
+        "type": "function",
+        "name": "allowance",
+        "source_mapping": {
+          "start": 319,
+          "length": 60,
+          "filename_used": "/home/travis/build/crytic/slither/tests/incorrect_erc20_interface.sol",
+          "filename_relative": "tests/incorrect_erc20_interface.sol",
+          "filename_absolute": "/home/travis/build/crytic/slither/tests/incorrect_erc20_interface.sol",
+          "filename_short": "tests/incorrect_erc20_interface.sol",
+          "lines": [
+            9
+          ],
+          "starting_column": 5,
+          "ending_column": 65
+        },
+        "contract": {
+          "type": "contract",
+          "name": "Token",
+          "source_mapping": {
+            "start": 26,
+            "length": 355,
+            "filename_used": "/home/travis/build/crytic/slither/tests/incorrect_erc20_interface.sol",
+            "filename_relative": "tests/incorrect_erc20_interface.sol",
+            "filename_absolute": "/home/travis/build/crytic/slither/tests/incorrect_erc20_interface.sol",
+            "filename_short": "tests/incorrect_erc20_interface.sol",
+            "lines": [
+              3,
+              4,
+              5,
+              6,
+              7,
+              8,
+              9,
+              10
+            ],
+            "starting_column": 1,
+            "ending_column": 2
+          }
+        }
+      },
+      {
+        "type": "function",
+        "name": "approve",
+        "source_mapping": {
+          "start": 102,
+          "length": 55,
+          "filename_used": "/home/travis/build/crytic/slither/tests/incorrect_erc20_interface.sol",
+          "filename_relative": "tests/incorrect_erc20_interface.sol",
+          "filename_absolute": "/home/travis/build/crytic/slither/tests/incorrect_erc20_interface.sol",
+          "filename_short": "tests/incorrect_erc20_interface.sol",
+          "lines": [
+            5
+          ],
+          "starting_column": 5,
+          "ending_column": 60
+        },
+        "contract": {
+          "type": "contract",
+          "name": "Token",
+          "source_mapping": {
+            "start": 26,
+            "length": 355,
+            "filename_used": "/home/travis/build/crytic/slither/tests/incorrect_erc20_interface.sol",
+            "filename_relative": "tests/incorrect_erc20_interface.sol",
+            "filename_absolute": "/home/travis/build/crytic/slither/tests/incorrect_erc20_interface.sol",
+            "filename_short": "tests/incorrect_erc20_interface.sol",
+            "lines": [
+              3,
+              4,
+              5,
+              6,
+              7,
+              8,
+              9,
+              10
+            ],
+            "starting_column": 1,
+            "ending_column": 2
+          }
+        }
+      },
+      {
+        "type": "function",
+        "name": "balanceOf",
+        "source_mapping": {
+          "start": 273,
+          "length": 41,
+          "filename_used": "/home/travis/build/crytic/slither/tests/incorrect_erc20_interface.sol",
+          "filename_relative": "tests/incorrect_erc20_interface.sol",
+          "filename_absolute": "/home/travis/build/crytic/slither/tests/incorrect_erc20_interface.sol",
+          "filename_short": "tests/incorrect_erc20_interface.sol",
+          "lines": [
+            8
+          ],
+          "starting_column": 5,
+          "ending_column": 46
+        },
+        "contract": {
+          "type": "contract",
+          "name": "Token",
+          "source_mapping": {
+            "start": 26,
+            "length": 355,
+            "filename_used": "/home/travis/build/crytic/slither/tests/incorrect_erc20_interface.sol",
+            "filename_relative": "tests/incorrect_erc20_interface.sol",
+            "filename_absolute": "/home/travis/build/crytic/slither/tests/incorrect_erc20_interface.sol",
+            "filename_short": "tests/incorrect_erc20_interface.sol",
+            "lines": [
+              3,
+              4,
+              5,
+              6,
+              7,
+              8,
+              9,
+              10
+            ],
+            "starting_column": 1,
+            "ending_column": 2
+          }
+        }
+      },
+      {
+        "type": "function",
+        "name": "totalSupply",
+        "source_mapping": {
+          "start": 236,
+          "length": 32,
+          "filename_used": "/home/travis/build/crytic/slither/tests/incorrect_erc20_interface.sol",
+          "filename_relative": "tests/incorrect_erc20_interface.sol",
+          "filename_absolute": "/home/travis/build/crytic/slither/tests/incorrect_erc20_interface.sol",
+          "filename_short": "tests/incorrect_erc20_interface.sol",
+          "lines": [
+            7
+          ],
+          "starting_column": 5,
+          "ending_column": 37
+        },
+        "contract": {
+          "type": "contract",
+          "name": "Token",
+          "source_mapping": {
+            "start": 26,
+            "length": 355,
+            "filename_used": "/home/travis/build/crytic/slither/tests/incorrect_erc20_interface.sol",
+            "filename_relative": "tests/incorrect_erc20_interface.sol",
+            "filename_absolute": "/home/travis/build/crytic/slither/tests/incorrect_erc20_interface.sol",
+            "filename_short": "tests/incorrect_erc20_interface.sol",
+            "lines": [
+              3,
+              4,
+              5,
+              6,
+              7,
+              8,
+              9,
+              10
+            ],
+            "starting_column": 1,
+            "ending_column": 2
+          }
+        }
+      },
      {
        "type": "function",
        "name": "transfer",
        "source_mapping": {
-          "start": 47,
+          "start": 46,
          "length": 51,
          "filename_used": "/home/travis/build/crytic/slither/tests/incorrect_erc20_interface.sol",
          "filename_relative": "tests/incorrect_erc20_interface.sol",
          "filename_absolute": "/home/travis/build/crytic/slither/tests/incorrect_erc20_interface.sol",
          "filename_short": "tests/incorrect_erc20_interface.sol",
          "lines": [
-            5
+            4
          ],
          "starting_column": 5,
          "ending_column": 56
@ -26,7 +190,7 @@
          "name": "Token",
          "source_mapping": {
            "start": 26,
-            "length": 75,
+            "length": 355,
            "filename_used": "/home/travis/build/crytic/slither/tests/incorrect_erc20_interface.sol",
            "filename_relative": "tests/incorrect_erc20_interface.sol",
            "filename_absolute": "/home/travis/build/crytic/slither/tests/incorrect_erc20_interface.sol",
@ -36,7 +200,51 @@
              4,
              5,
              6,
-              7
+              7,
+              8,
+              9,
+              10
+            ],
+            "starting_column": 1,
+            "ending_column": 2
+          }
+        }
+      },
+      {
+        "type": "function",
+        "name": "transferFrom",
+        "source_mapping": {
+          "start": 162,
+          "length": 69,
+          "filename_used": "/home/travis/build/crytic/slither/tests/incorrect_erc20_interface.sol",
+          "filename_relative": "tests/incorrect_erc20_interface.sol",
+          "filename_absolute": "/home/travis/build/crytic/slither/tests/incorrect_erc20_interface.sol",
+          "filename_short": "tests/incorrect_erc20_interface.sol",
+          "lines": [
+            6
+          ],
+          "starting_column": 5,
+          "ending_column": 74
+        },
+        "contract": {
+          "type": "contract",
+          "name": "Token",
+          "source_mapping": {
+            "start": 26,
+            "length": 355,
+            "filename_used": "/home/travis/build/crytic/slither/tests/incorrect_erc20_interface.sol",
+            "filename_relative": "tests/incorrect_erc20_interface.sol",
+            "filename_absolute": "/home/travis/build/crytic/slither/tests/incorrect_erc20_interface.sol",
+            "filename_short": "tests/incorrect_erc20_interface.sol",
+            "lines": [
+              3,
+              4,
+              5,
+              6,
+              7,
+              8,
+              9,
+              10
            ],
            "starting_column": 1,
            "ending_column": 2
--- a/tests/expected_json/incorrect_erc20_interface.erc20-interface.txt
+++ b/tests/expected_json/incorrect_erc20_interface.erc20-interface.txt
@ -1,5 +1,10 @@
 INFO:Detectors:[93m
-Token (tests/incorrect_erc20_interface.sol#3-7) has incorrect ERC20 function interface(s):
-	-transfer (tests/incorrect_erc20_interface.sol#5)
+Token (tests/incorrect_erc20_interface.sol#3-10) has incorrect ERC20 function interface(s):
+	-transfer (tests/incorrect_erc20_interface.sol#4)
+	-approve (tests/incorrect_erc20_interface.sol#5)
+	-transferFrom (tests/incorrect_erc20_interface.sol#6)
+	-totalSupply (tests/incorrect_erc20_interface.sol#7)
+	-balanceOf (tests/incorrect_erc20_interface.sol#8)
+	-allowance (tests/incorrect_erc20_interface.sol#9)
 Reference: https://github.com/crytic/slither/wiki/Detector-Documentation#incorrect-erc20-interface[0m
 INFO:Slither:tests/incorrect_erc20_interface.sol analyzed (1 contracts), 1 result(s) found