From 13078d4766e1435baa58e39d60de5faa23dacfeb Mon Sep 17 00:00:00 2001 From: Eshaan Bansal Date: Wed, 6 Mar 2024 14:58:55 +0530 Subject: [PATCH] feat: update slither to 0.10.1 (#31) Resolves: ENG-1055. Related: https://github.com/crytic/slither/pull/2344 --- .../.deepsource/analyzer/analyzer.toml | 2 +- .../.deepsource/issues/SLITHER-W1023.toml | 2 +- .../.deepsource/issues/SLITHER-W1026.toml | 2 +- .../.deepsource/issues/SLITHER-W1056.toml | 2 +- .../.deepsource/issues/SLITHER-W1093.toml | 64 +++++++++++++++++++ analyzers/slither/utils/issue_map.json | 3 + analyzers/slither/utils/issue_map_gen.py | 4 +- 7 files changed, 73 insertions(+), 6 deletions(-) create mode 100644 analyzers/slither/.deepsource/issues/SLITHER-W1093.toml diff --git a/analyzers/slither/.deepsource/analyzer/analyzer.toml b/analyzers/slither/.deepsource/analyzer/analyzer.toml index a250c3fe..27a336bf 100644 --- a/analyzers/slither/.deepsource/analyzer/analyzer.toml +++ b/analyzers/slither/.deepsource/analyzer/analyzer.toml @@ -3,5 +3,5 @@ category = "lang" name = "Slither" shortcode = "slither" status = "active" -tool_latest_version = "0.10.0" +tool_latest_version = "0.10.1" description = "Slither is a Solidity & Vyper static analysis framework developed by Crytic, a blockchain security group by Trail of Bits." diff --git a/analyzers/slither/.deepsource/issues/SLITHER-W1023.toml b/analyzers/slither/.deepsource/issues/SLITHER-W1023.toml index 11b2bca0..84867bb2 100644 --- a/analyzers/slither/.deepsource/issues/SLITHER-W1023.toml +++ b/analyzers/slither/.deepsource/issues/SLITHER-W1023.toml @@ -31,5 +31,5 @@ The function will return 6 bytes starting from offset 5, instead of returning a Use the `leave` statement. ## Learn more -[incorrect-return](https://github.com/crytic/slither/wiki/Detector-Documentation#incorrect-assembly-return) on Slither's wiki. +[incorrect-return](https://github.com/crytic/slither/wiki/Detector-Documentation#incorrect-return-in-assembly) on Slither's wiki. """ diff --git a/analyzers/slither/.deepsource/issues/SLITHER-W1026.toml b/analyzers/slither/.deepsource/issues/SLITHER-W1026.toml index 04d4fbdf..2795bf88 100644 --- a/analyzers/slither/.deepsource/issues/SLITHER-W1026.toml +++ b/analyzers/slither/.deepsource/issues/SLITHER-W1026.toml @@ -26,5 +26,5 @@ The function will halt the execution, instead of returning a two uint. Use the `leave` statement. ## Learn more -[return-leave](https://github.com/crytic/slither/wiki/Detector-Documentation#incorrect-assembly-return) on Slither's wiki. +[return-leave](https://github.com/crytic/slither/wiki/Detector-Documentation#return-instead-of-leave-in-assembly) on Slither's wiki. """ diff --git a/analyzers/slither/.deepsource/issues/SLITHER-W1056.toml b/analyzers/slither/.deepsource/issues/SLITHER-W1056.toml index 222c9871..7428882a 100644 --- a/analyzers/slither/.deepsource/issues/SLITHER-W1056.toml +++ b/analyzers/slither/.deepsource/issues/SLITHER-W1056.toml @@ -13,7 +13,7 @@ Detects the possible usage of a variable before the declaration is stepped over ```solidity contract C { function f(uint z) public returns (uint) { - uint y = x + 9 + z; // 'z' is used pre-declaration + uint y = x + 9 + z; // 'x' is used pre-declaration uint x = 7; if (z % 2 == 0) { diff --git a/analyzers/slither/.deepsource/issues/SLITHER-W1093.toml b/analyzers/slither/.deepsource/issues/SLITHER-W1093.toml new file mode 100644 index 00000000..a35de792 --- /dev/null +++ b/analyzers/slither/.deepsource/issues/SLITHER-W1093.toml @@ -0,0 +1,64 @@ +title = "Out-of-order retryable transactions" +verbose_name = "out-of-order-retryable" +severity = "major" +category = "antipattern" +weight = 60 +description = """ +Out-of-order retryable transactions + + + +## Exploit Scenario + +```solidity +contract L1 { + function doStuffOnL2() external { + // Retryable A + IInbox(inbox).createRetryableTicket({ + to: l2contract, + l2CallValue: 0, + maxSubmissionCost: maxSubmissionCost, + excessFeeRefundAddress: msg.sender, + callValueRefundAddress: msg.sender, + gasLimit: gasLimit, + maxFeePerGas: maxFeePerGas, + data: abi.encodeCall(l2contract.claim_rewards, ()) + }); + // Retryable B + IInbox(inbox).createRetryableTicket({ + to: l2contract, + l2CallValue: 0, + maxSubmissionCost: maxSubmissionCost, + excessFeeRefundAddress: msg.sender, + callValueRefundAddress: msg.sender, + gasLimit: gas, + maxFeePerGas: maxFeePerGas, + data: abi.encodeCall(l2contract.unstake, ()) + }); + } +} + +contract L2 { + function claim_rewards() public { + // rewards is computed based on balance and staking period + uint unclaimed_rewards = _compute_and_update_rewards(); + token.safeTransfer(msg.sender, unclaimed_rewards); + } + + // Call claim_rewards before unstaking, otherwise you lose your rewards + function unstake() public { + _free_rewards(); // clean up rewards related variables + balance = balance[msg.sender]; + balance[msg.sender] = 0; + staked_token.safeTransfer(msg.sender, balance); + } +} +``` +Bob calls `doStuffOnL2` but the first retryable ticket calling `claim_rewards` fails. The second retryable ticket calling `unstake` is executed successfully. As a result, Bob loses his rewards. + +## Recommendation +Do not rely on the order or successful execution of retryable tickets. + +## Learn more +[out-of-order-retryable](https://github.com/crytic/slither/wiki/Detector-Documentation#out-of-order-retryable-transactions) on Slither's wiki. +""" diff --git a/analyzers/slither/utils/issue_map.json b/analyzers/slither/utils/issue_map.json index ad37f3d6..a9bd0775 100644 --- a/analyzers/slither/utils/issue_map.json +++ b/analyzers/slither/utils/issue_map.json @@ -274,5 +274,8 @@ }, "4-0-var-read-using-this": { "issue_code": "SLITHER-W1092" + }, + "1-1-out-of-order-retryable": { + "issue_code": "SLITHER-W1093" } } \ No newline at end of file diff --git a/analyzers/slither/utils/issue_map_gen.py b/analyzers/slither/utils/issue_map_gen.py index a25d5b0b..324b6c37 100644 --- a/analyzers/slither/utils/issue_map_gen.py +++ b/analyzers/slither/utils/issue_map_gen.py @@ -1,6 +1,6 @@ import itertools import json -from typing import Dict, Generator +from typing import Dict, Iterator from constants import ISSUE_MAP_FILE, ISSUE_PREFIX from detectors import get_all_detector_json @@ -8,7 +8,7 @@ __all__ = ["get_issue_map", "generate_mapping"] -def _get_next_code(mapping: Dict[str, Dict[str, str]]) -> Generator[int]: +def _get_next_code(mapping: Dict[str, Dict[str, str]]) -> Iterator[int]: """Return the next available issue code.""" num_issues = len(mapping.keys()) # get the number of issues already in the mapping next_code = 1001 + num_issues # issue code series starts from `1001`