Detector for encodeWithSelector wrong argument count/ mismatched types

slither/detectors/all_detectors.py

@@ -84,3 +84,4 @@
 from .statements.msg_value_in_loop import MsgValueInLoop
 from .statements.delegatecall_in_loop import DelegatecallInLoop
 from .functions.protected_variable import ProtectedVariables
+from .compiler_bugs.wrong_selector_with_selector import WrongEncodeWithSelector

slither/detectors/compiler_bugs/wrong_selector_with_selector.py

@@ -0,0 +1,118 @@
+from slither.detectors.abstract_detector import AbstractDetector, DetectorClassification
+from slither.slithir.operations import SolidityCall
+from slither.core.declarations.solidity_variables import SolidityFunction
+from slither.slithir.operations.assignment import Assignment
+from slither.slithir.variables.reference import ReferenceVariable
+from slither.slithir.variables.constant import Constant
+from slither.utils.function import get_function_id
+
+
+def get_signatures(c):
+    """Build a dictionary mapping function ids to signature, name, arguments for a contract"""
+    result = {}
+    functions = c.functions
+    for f in functions:
+        if f.visibility not in ["public", "external"]:
+            continue
+        if f.is_constructor or f.is_fallback:
+            continue
+        result[get_function_id(f.full_name)] = (f.full_name, *f.signature[:2])
+
+    variables = c.state_variables
+    for variable in variables:
+        if variable.visibility not in ["public"]:
+            continue
+        name = variable.full_name
+        result[get_function_id(name)] = (name, (),())
+
+    return result
+
+
+class WrongEncodeWithSelector(AbstractDetector):
+    """
+    Detect calls to abi.encodeWithSelector that may result in unexpected calldata encodings
+    """
+
+    ARGUMENT = "wrongencodeselector"
+    HELP = "abi.encodeWithSelector with unexpected arguments"
+    IMPACT = DetectorClassification.MEDIUM
+    CONFIDENCE = DetectorClassification.HIGH
+
+    WIKI = "https://github.com/trailofbits/slither/wiki/encode-with-selector"
+    WIKI_TITLE = "Encode With Selector uses unexpected parameters"
+    WIKI_DESCRIPTION = "Plugin example"
+    WIKI_EXPLOIT_SCENARIO = """
+```solidity
+contract Test {   
+    event Val(uint, uint);
+    function f(uint a, uint b) public {
+        emit Val(a, b);
+    }
+}
+contract D {
+    function g() public {
+        Test t = new Test();
+        address(t).call(abi.encodeWithSelector(Test.f.selector, "test"));
+    }
+}
+```
+The compiler will not check_contract if the parameters of abi.encodeWithSelector match the arguments expected at the destination 
+function signature.
+"""
+    WIKI_RECOMMENDATION = "Make sure that encodeWithSelector is building a calldata that matches the target function signature"
+
+    def _detect(self):
+        #gather all known funcids
+        func_ids = {}
+        for contract in self.contracts:
+            func_ids.update(get_signatures(contract))
+        #todo: include func_ids from the public db
+
+        results = []
+        for contract in self.contracts:
+            for func, node in check_contract(contract, func_ids):
+                info = [func, " calls abi.encodeWithSelector() with wrong arguments at", node ]
+                json = self.generate_result(info)
+                results.append(json)
+
+        return results
+
+def check_ir(function, node, ir, func_ids):
+    result = []
+    if isinstance(ir, SolidityCall) and ir.function == SolidityFunction(
+            "abi.encodeWithSelector()"
+    ):
+
+        # build reference bindings dict
+        assignments = {}
+        for ir1 in node.irs:
+            if isinstance(ir1, Assignment):
+                assignments[ir1.lvalue.name] = ir1.rvalue
+
+        # if the selector is a reference, deref
+        selector = ir.arguments[0]
+        if isinstance(selector, ReferenceVariable):
+            selector = assignments[selector.name]
+
+        assert isinstance(selector, Constant)
+
+        _, _, argument_types = func_ids[selector.value]
+        arguments = ir.arguments[1:]
+
+        if len(argument_types) != len(arguments):
+            result.append((function, node))
+
+        # Todo check_contract unmatching argument types for correct count
+    return result
+
+def check_contract(contract, func_ids):
+    """ check_contract if contract has an ecodeWhitSelector that uses a selector
+        for a method with an unmatching number of arguments
+    """
+    result = []
+    for function in contract.functions_and_modifiers_declared:
+        for node in function.nodes:
+            for ir in node.irs:
+                result += check_ir(function, node, ir, func_ids)
+
+    return result

tests/detectors/wrongencodeselector/0.8.0/wrongencodeselector.sol

@@ -0,0 +1,17 @@
+contract Test {   
+    event Val(uint, uint);
+    function f(uint a, uint b) public {
+        emit Val(a, b);
+    }
+}
+contract D {
+    function bad() public {
+        Test t = new Test();
+        address(t).call(abi.encodeWithSelector(Test.f.selector,"test"));
+    }
+    function good() public {
+        Test t = new Test();
+        address(t).call(abi.encodeWithSelector(Test.f.selector, 1, 2));
+    }
+}
+

tests/detectors/wrongencodeselector/0.8.0/wrongencodeselector.sol.0.8.0.WrongEncodeWithSelector.json

@@ -0,0 +1,137 @@
+[
+    [
+        {
+            "elements": [
+                {
+                    "type": "function",
+                    "name": "bad",
+                    "source_mapping": {
+                        "start": 135,
+                        "length": 131,
+                        "filename_used": "/GENERIC_PATH",
+                        "filename_relative": "tests/detectors/wrongencodeselector/0.8.0/wrongencodeselector.sol",
+                        "filename_absolute": "/GENERIC_PATH",
+                        "filename_short": "tests/detectors/wrongencodeselector/0.8.0/wrongencodeselector.sol",
+                        "is_dependency": false,
+                        "lines": [
+                            8,
+                            9,
+                            10,
+                            11
+                        ],
+                        "starting_column": 5,
+                        "ending_column": 6
+                    },
+                    "type_specific_fields": {
+                        "parent": {
+                            "type": "contract",
+                            "name": "D",
+                            "source_mapping": {
+                                "start": 118,
+                                "length": 286,
+                                "filename_used": "/GENERIC_PATH",
+                                "filename_relative": "tests/detectors/wrongencodeselector/0.8.0/wrongencodeselector.sol",
+                                "filename_absolute": "/GENERIC_PATH",
+                                "filename_short": "tests/detectors/wrongencodeselector/0.8.0/wrongencodeselector.sol",
+                                "is_dependency": false,
+                                "lines": [
+                                    7,
+                                    8,
+                                    9,
+                                    10,
+                                    11,
+                                    12,
+                                    13,
+                                    14,
+                                    15,
+                                    16
+                                ],
+                                "starting_column": 1,
+                                "ending_column": 2
+                            }
+                        },
+                        "signature": "bad()"
+                    }
+                },
+                {
+                    "type": "node",
+                    "name": "address(t).call(abi.encodeWithSelector(Test.f.selector,test))",
+                    "source_mapping": {
+                        "start": 196,
+                        "length": 63,
+                        "filename_used": "/GENERIC_PATH",
+                        "filename_relative": "tests/detectors/wrongencodeselector/0.8.0/wrongencodeselector.sol",
+                        "filename_absolute": "/GENERIC_PATH",
+                        "filename_short": "tests/detectors/wrongencodeselector/0.8.0/wrongencodeselector.sol",
+                        "is_dependency": false,
+                        "lines": [
+                            10
+                        ],
+                        "starting_column": 9,
+                        "ending_column": 72
+                    },
+                    "type_specific_fields": {
+                        "parent": {
+                            "type": "function",
+                            "name": "bad",
+                            "source_mapping": {
+                                "start": 135,
+                                "length": 131,
+                                "filename_used": "/GENERIC_PATH",
+                                "filename_relative": "tests/detectors/wrongencodeselector/0.8.0/wrongencodeselector.sol",
+                                "filename_absolute": "/GENERIC_PATH",
+                                "filename_short": "tests/detectors/wrongencodeselector/0.8.0/wrongencodeselector.sol",
+                                "is_dependency": false,
+                                "lines": [
+                                    8,
+                                    9,
+                                    10,
+                                    11
+                                ],
+                                "starting_column": 5,
+                                "ending_column": 6
+                            },
+                            "type_specific_fields": {
+                                "parent": {
+                                    "type": "contract",
+                                    "name": "D",
+                                    "source_mapping": {
+                                        "start": 118,
+                                        "length": 286,
+                                        "filename_used": "/GENERIC_PATH",
+                                        "filename_relative": "tests/detectors/wrongencodeselector/0.8.0/wrongencodeselector.sol",
+                                        "filename_absolute": "/GENERIC_PATH",
+                                        "filename_short": "tests/detectors/wrongencodeselector/0.8.0/wrongencodeselector.sol",
+                                        "is_dependency": false,
+                                        "lines": [
+                                            7,
+                                            8,
+                                            9,
+                                            10,
+                                            11,
+                                            12,
+                                            13,
+                                            14,
+                                            15,
+                                            16
+                                        ],
+                                        "starting_column": 1,
+                                        "ending_column": 2
+                                    }
+                                },
+                                "signature": "bad()"
+                            }
+                        }
+                    }
+                }
+            ],
+            "description": "D.bad() (tests/detectors/wrongencodeselector/0.8.0/wrongencodeselector.sol#8-11) calls abi.encodeWithSelector() with wrong arguments ataddress(t).call(abi.encodeWithSelector(Test.f.selector,test)) (tests/detectors/wrongencodeselector/0.8.0/wrongencodeselector.sol#10)",
+            "markdown": "[D.bad()](tests/detectors/wrongencodeselector/0.8.0/wrongencodeselector.sol#L8-L11) calls abi.encodeWithSelector() with wrong arguments at[address(t).call(abi.encodeWithSelector(Test.f.selector,test))](tests/detectors/wrongencodeselector/0.8.0/wrongencodeselector.sol#L10)",
+            "first_markdown_element": "tests/detectors/wrongencodeselector/0.8.0/wrongencodeselector.sol#L8-L11",
+            "id": "9230578efcdc63464cf775b07aaf1d04e77ea58363f4809b7f6ffb1dcd46e34f",
+            "check": "wrongencodeselector",
+            "impact": "Medium",
+            "confidence": "High"
+        }
+    ]
+]