Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add Gelato VRF unprotected request detector #2582

Merged
merged 5 commits into from
Oct 10, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions slither/detectors/all_detectors.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
78 changes: 78 additions & 0 deletions slither/detectors/functions/gelato_unprotected_randomness.py
Original file line number Diff line number Diff line change
@@ -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
Original file line number Diff line number Diff line change
@@ -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)

Original file line number Diff line number Diff line change
@@ -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));
}

}
Binary file not shown.
5 changes: 5 additions & 0 deletions tests/e2e/detectors/test_detectors.py
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down