Merge pull request #2582 from crytic/dev-gelato-vrf-detector

Add Gelato VRF unprotected request detector

slither/detectors/all_detectors.py

@@ -97,6 +97,7 @@
 from .statements.tautological_compare import TautologicalCompare
 from .statements.return_bomb import ReturnBomb
 from .functions.out_of_order_retryable import OutOfOrderRetryable
+from .functions.gelato_unprotected_randomness import GelatoUnprotectedRandomness
 from .statements.chronicle_unchecked_price import ChronicleUncheckedPrice
 from .statements.pyth_unchecked_confidence import PythUncheckedConfidence
 from .statements.pyth_unchecked_publishtime import PythUncheckedPublishTime

slither/detectors/functions/gelato_unprotected_randomness.py

@@ -0,0 +1,78 @@
+from typing import List
+
+from slither.slithir.operations.internal_call import InternalCall
+from slither.detectors.abstract_detector import (
+    AbstractDetector,
+    DetectorClassification,
+    DETECTOR_INFO,
+)
+from slither.utils.output import Output
+
+
+class GelatoUnprotectedRandomness(AbstractDetector):
+    """
+    Unprotected Gelato VRF requests
+    """
+
+    ARGUMENT = "gelato-unprotected-randomness"
+    HELP = "Call to _requestRandomness within an unprotected function"
+    IMPACT = DetectorClassification.MEDIUM
+    CONFIDENCE = DetectorClassification.MEDIUM
+
+    WIKI = "https://github.com/crytic/slither/wiki/Detector-Documentation#gelato-unprotected-randomness"
+
+    WIKI_TITLE = "Gelato unprotected randomness"
+    WIKI_DESCRIPTION = "Detect calls to `_requestRandomness` within an unprotected function."
+
+    # region wiki_exploit_scenario
+    WIKI_EXPLOIT_SCENARIO = """
+```solidity
+contract C is GelatoVRFConsumerBase {
+    function _fulfillRandomness(
+        uint256 randomness,
+        uint256,
+        bytes memory extraData
+    ) internal override {
+        // Do something with the random number
+    }
+
+    function bad() public {
+        _requestRandomness(abi.encode(msg.sender));
+    }
+}
+```
+The function `bad` is uprotected and requests randomness."""
+    # endregion wiki_exploit_scenario
+
+    WIKI_RECOMMENDATION = (
+        "Function that request randomness should be allowed only to authorized users."
+    )
+
+    def _detect(self) -> List[Output]:
+        results = []
+
+        for contract in self.compilation_unit.contracts_derived:
+            if "GelatoVRFConsumerBase" in [c.name for c in contract.inheritance]:
+                for function in contract.functions_entry_points:
+                    if not function.is_protected() and (
+                        nodes_request := [
+                            ir.node
+                            for ir in function.all_internal_calls()
+                            if isinstance(ir, InternalCall)
+                            and ir.function_name == "_requestRandomness"
+                        ]
+                    ):
+                        # Sort so output is deterministic
+                        nodes_request.sort(key=lambda x: (x.node_id, x.function.full_name))
+
+                        for node in nodes_request:
+                            info: DETECTOR_INFO = [
+                                function,
+                                " is unprotected and request randomness from Gelato VRF\n\t- ",
+                                node,
+                                "\n",
+                            ]
+                            res = self.generate_result(info)
+                            results.append(res)
+
+        return results

tests/e2e/detectors/snapshots/detectors__detector_GelatoUnprotectedRandomness_0_8_20_gelato_unprotected_randomness_sol__0.txt

@@ -0,0 +1,6 @@
+C.bad() (tests/e2e/detectors/test_data/gelato-unprotected-randomness/0.8.20/gelato_unprotected_randomness.sol#42-44) is unprotected and request randomness from Gelato VRF
+	- id = _requestRandomness(abi.encode(msg.sender)) (tests/e2e/detectors/test_data/gelato-unprotected-randomness/0.8.20/gelato_unprotected_randomness.sol#43)
+
+C.good2() (tests/e2e/detectors/test_data/gelato-unprotected-randomness/0.8.20/gelato_unprotected_randomness.sol#51-54) is unprotected and request randomness from Gelato VRF
+	- id = _requestRandomness(abi.encode(msg.sender)) (tests/e2e/detectors/test_data/gelato-unprotected-randomness/0.8.20/gelato_unprotected_randomness.sol#53)
+

tests/e2e/detectors/test_data/gelato-unprotected-randomness/0.8.20/gelato_unprotected_randomness.sol

@@ -0,0 +1,62 @@
+// Mock GelatoVRFConsumerBase for what we need
+abstract contract GelatoVRFConsumerBase {
+    bool[] public requestPending;
+    mapping(uint256 => bytes32) public requestedHash;
+
+    function _fulfillRandomness(
+        uint256 randomness,
+        uint256 requestId,
+        bytes memory extraData
+    ) internal virtual;
+
+    function _requestRandomness(
+        bytes memory extraData
+    ) internal returns (uint256 requestId) {
+        requestId = uint256(requestPending.length);
+        requestPending.push();
+        requestPending[requestId] = true;
+
+        bytes memory data = abi.encode(requestId, extraData);
+        uint256 round = 111;
+
+        bytes memory dataWithRound = abi.encode(round, data);
+        bytes32 requestHash = keccak256(dataWithRound);
+
+        requestedHash[requestId] = requestHash;
+    }
+
+}
+
+contract C is GelatoVRFConsumerBase {
+    address owner;
+    mapping(address => bool) authorized;
+
+    function _fulfillRandomness(
+        uint256 randomness,
+        uint256,
+        bytes memory extraData
+    ) internal override {
+        // Do something with the random number
+    }
+
+    function bad() public {
+        uint id = _requestRandomness(abi.encode(msg.sender));
+    }
+
+    function good() public {
+        require(msg.sender == owner);
+        uint id = _requestRandomness(abi.encode(msg.sender));
+    }
+
+    // This is currently a FP due to the limitation of function.is_protected
+    function good2() public {
+        require(authorized[msg.sender]);
+        uint id = _requestRandomness(abi.encode(msg.sender));
+    }
+
+    function good3() public {
+        if (msg.sender != owner) { revert(); }
+        uint id = _requestRandomness(abi.encode(msg.sender));
+    }
+
+}

tests/e2e/detectors/test_detectors.py

@@ -1714,6 +1714,11 @@ def id_test(test_item: Test):
         "out_of_order_retryable.sol",
         "0.8.20",
     ),
+    Test(
+        all_detectors.GelatoUnprotectedRandomness,
+        "gelato_unprotected_randomness.sol",
+        "0.8.20",
+    ),
     Test(
         all_detectors.ChronicleUncheckedPrice,
         "chronicle_unchecked_price.sol",