Merge pull request #1845 from crytic/detector/encode-packed-collision

Detector/encode packed collision

slither/detectors/all_detectors.py

@@ -89,3 +89,4 @@
 from .functions.permit_domain_signature_collision import DomainSeparatorCollision
 from .functions.codex import Codex
 from .functions.cyclomatic_complexity import CyclomaticComplexity
+from .operations.encode_packed import EncodePackedCollision

slither/detectors/operations/encode_packed.py

@@ -0,0 +1,104 @@
+"""
+Module detecting usage of more than one dynamic type in abi.encodePacked() arguments which could lead to collision
+"""
+
+from slither.detectors.abstract_detector import AbstractDetector, DetectorClassification
+from slither.core.declarations.solidity_variables import SolidityFunction
+from slither.slithir.operations import SolidityCall
+from slither.analyses.data_dependency.data_dependency import is_tainted
+from slither.core.solidity_types import ElementaryType
+from slither.core.solidity_types import ArrayType
+
+
+def _is_dynamic_type(arg):
+    """
+    Args:
+        arg (function argument)
+    Returns:
+        Bool
+    """
+    if isinstance(arg.type, ElementaryType) and (arg.type.name in ["string", "bytes"]):
+        return True
+    if isinstance(arg.type, ArrayType) and arg.type.length is None:
+        return True
+
+    return False
+
+
+def _detect_abi_encodePacked_collision(contract):
+    """
+    Args:
+        contract (Contract)
+    Returns:
+        list((Function), (list (Node)))
+    """
+    ret = []
+    # pylint: disable=too-many-nested-blocks
+    for f in contract.functions_and_modifiers_declared:
+        for n in f.nodes:
+            for ir in n.irs:
+                if isinstance(ir, SolidityCall) and ir.function == SolidityFunction(
+                    "abi.encodePacked()"
+                ):
+                    dynamic_type_count = 0
+                    for arg in ir.arguments:
+                        if is_tainted(arg, contract) and _is_dynamic_type(arg):
+                            dynamic_type_count += 1
+                        elif dynamic_type_count > 1:
+                            ret.append((f, n))
+                            dynamic_type_count = 0
+                        else:
+                            dynamic_type_count = 0
+                    if dynamic_type_count > 1:
+                        ret.append((f, n))
+    return ret
+
+
+class EncodePackedCollision(AbstractDetector):
+    """
+    Detect usage of more than one dynamic type in abi.encodePacked() arguments which could to collision
+    """
+
+    ARGUMENT = "encode-packed-collision"
+    HELP = "ABI encodePacked Collision"
+    IMPACT = DetectorClassification.HIGH
+    CONFIDENCE = DetectorClassification.HIGH
+
+    WIKI = (
+        "https://github.com/crytic/slither/wiki/Detector-Documentation#abi-encodePacked-collision"
+    )
+
+    WIKI_TITLE = "ABI encodePacked Collision"
+    WIKI_DESCRIPTION = """Detect collision due to dynamic type usages in `abi.encodePacked`"""
+
+    WIKI_EXPLOIT_SCENARIO = """
+```solidity
+contract Sign {
+    function get_hash_for_signature(string name, string doc) external returns(bytes32) {
+        return keccak256(abi.encodePacked(name, doc));
+    }
+}
+```
+Bob calls `get_hash_for_signature` with (`bob`, `This is the content`). The hash returned is used as an ID.
+Eve creates a collision with the ID using (`bo`, `bThis is the content`) and compromises the system.
+"""
+    WIKI_RECOMMENDATION = """Do not use more than one dynamic type in `abi.encodePacked()`
+(see the [Solidity documentation](https://solidity.readthedocs.io/en/v0.5.10/abi-spec.html?highlight=abi.encodePacked#non-standard-packed-modeDynamic)). 
+Use `abi.encode()`, preferably."""
+
+    def _detect(self):
+        """Detect usage of more than one dynamic type in abi.encodePacked(..) arguments which could lead to collision"""
+        results = []
+        for c in self.compilation_unit.contracts:
+            values = _detect_abi_encodePacked_collision(c)
+            for func, node in values:
+                info = [
+                    func,
+                    " calls abi.encodePacked() with multiple dynamic arguments:\n\t- ",
+                    node,
+                    "\n",
+                ]
+                json = self.generate_result(info)
+                results.append(json)
+
+        return results

tests/e2e/detectors/snapshots/detectors__detector_EncodePackedCollision_0_7_6_encode_packed_collision_sol__0.txt

@@ -0,0 +1,15 @@
+EncodePackedCollision.bad4(bytes,bytes) (tests/e2e/detectors/test_data/encode-packed-collision/0.7.6/encode_packed_collision.sol#34-36) calls abi.encodePacked() with multiple dynamic arguments:
+	- packed = abi.encodePacked(a,a2,a3,a) (tests/e2e/detectors/test_data/encode-packed-collision/0.7.6/encode_packed_collision.sol#35)
+
+EncodePackedCollision.bad2(string,uint256[]) (tests/e2e/detectors/test_data/encode-packed-collision/0.7.6/encode_packed_collision.sol#24-26) calls abi.encodePacked() with multiple dynamic arguments:
+	- packed = abi.encodePacked(stra,arra) (tests/e2e/detectors/test_data/encode-packed-collision/0.7.6/encode_packed_collision.sol#25)
+
+EncodePackedCollision.bad3_get_hash_for_signature(string,string) (tests/e2e/detectors/test_data/encode-packed-collision/0.7.6/encode_packed_collision.sol#29-31) calls abi.encodePacked() with multiple dynamic arguments:
+	- keccak256(bytes)(abi.encodePacked(name,doc)) (tests/e2e/detectors/test_data/encode-packed-collision/0.7.6/encode_packed_collision.sol#30)
+
+EncodePackedCollision.bad0(string,string) (tests/e2e/detectors/test_data/encode-packed-collision/0.7.6/encode_packed_collision.sol#14-16) calls abi.encodePacked() with multiple dynamic arguments:
+	- packed = abi.encodePacked(stra,strb) (tests/e2e/detectors/test_data/encode-packed-collision/0.7.6/encode_packed_collision.sol#15)
+
+EncodePackedCollision.bad1(string,bytes) (tests/e2e/detectors/test_data/encode-packed-collision/0.7.6/encode_packed_collision.sol#19-21) calls abi.encodePacked() with multiple dynamic arguments:
+	- packed = abi.encodePacked(stra,bytesa) (tests/e2e/detectors/test_data/encode-packed-collision/0.7.6/encode_packed_collision.sol#20)
+

tests/e2e/detectors/test_data/encode-packed-collision/0.7.6/encode_packed_collision.sol

@@ -0,0 +1,78 @@
+contract ABIencodePacked{
+
+  uint a;
+  string str1 = "a";
+  string str2 = "bc";
+  bytes _bytes = "hello world";
+  uint[] arr;
+  uint[2] arr2;
+  string[3] str_arr3; /* This nested dynamic type is not supported in abi.encodePacked mode by solc */
+  string[] str_array; /* This nested dynamic type is not supported in abi.encodePacked mode by solc */
+  bytes[] bytes_array; /* This nested dynamic type and tuples are not supported in abi.encodePacked mode by solc */
+
+  /* Two dynamic types */
+  function bad0(string calldata stra, string calldata strb) external{
+    bytes memory packed = abi.encodePacked(stra, strb);
+  }
+
+  /* Two dynamic types */
+  function bad1(string calldata stra, bytes calldata bytesa) external{
+    bytes memory packed = abi.encodePacked(stra, bytesa);
+  }
+
+  /* Two dynamic types */
+  function bad2(string calldata stra, uint[] calldata arra) external{
+    bytes memory packed = abi.encodePacked(stra, arra);
+  }
+
+  /* Two dynamic types */
+  function bad3_get_hash_for_signature(string calldata name, string calldata doc) external returns (bytes32) {                              
+        return keccak256(abi.encodePacked(name, doc));
+  }
+
+  /* Two dynamic types between non dynamic types */
+  function bad4(bytes calldata a2, bytes calldata a3) external {
+    bytes memory packed = abi.encodePacked(a, a2, a3, a);
+  }
+
+  /* Two dynamic types but static values*/
+  function good0() external{
+    bytes memory packed = abi.encodePacked(str1, str2);
+  }
+
+  /* Two dynamic types but static values*/
+  function good1() external{
+    bytes memory packed = abi.encodePacked(str1, _bytes);
+  }
+
+  /* Two dynamic types but static values*/
+  function good2() external{
+    bytes memory packed = abi.encodePacked(str1, arr);
+  }
+
+  /* No dynamic types */
+  function good3() external{
+    bytes memory packed = abi.encodePacked(a);
+  }
+
+  /* One dynamic type */
+  function good4() external{
+    bytes memory packed = abi.encodePacked(str1);
+  }
+
+  /* One dynamic type */
+  function good5() external{
+    bytes memory packed = abi.encodePacked(a, str1);
+  }
+
+  /* One dynamic type */
+  function good6() external{
+    bytes memory packed = abi.encodePacked(str1, arr2);
+  }
+
+  /* Two dynamic types but not consecutive*/
+  function good7(string calldata a, uint b, string calldata c) external{
+    bytes memory packed = abi.encodePacked(a, b, c);
+  }
+}
+

tests/e2e/detectors/test_detectors.py

@@ -1639,6 +1639,11 @@ def id_test(test_item: Test):
         "LowCyclomaticComplexity.sol",
         "0.8.16",
     ),
+    Test(
+        all_detectors.EncodePackedCollision,
+        "encode_packed_collision.sol",
+        "0.7.6",
+    ),
 ]
 
 GENERIC_PATH = "/GENERIC_PATH"