From c894abe0dc7f8fe829e9d58958f43615d7d455e2 Mon Sep 17 00:00:00 2001 From: Nikita Kirillov Date: Wed, 18 Oct 2023 11:00:40 +0500 Subject: [PATCH 01/48] Initialized balancer detector --- docs/balancer/readonly_reentrancy.md | 19 +++++++++++++++++++ .../detectors/balancer/readonly_reentrancy.py | 0 tests/balancer/readonly_reentrancy_test.sol | 0 3 files changed, 19 insertions(+) create mode 100644 docs/balancer/readonly_reentrancy.md create mode 100644 slither_pess/detectors/balancer/readonly_reentrancy.py create mode 100644 tests/balancer/readonly_reentrancy_test.sol diff --git a/docs/balancer/readonly_reentrancy.md b/docs/balancer/readonly_reentrancy.md new file mode 100644 index 0000000..4689a1c --- /dev/null +++ b/docs/balancer/readonly_reentrancy.md @@ -0,0 +1,19 @@ +# Balancer Readonly Reentrancy + +## Configuration +* Check: `bal-readonly-reentrancy` +* Severity: `High` +* Confidence: `Medium` + +## Description +Highlights the use of Balancer getter functions `getRate` and `getPoolTokens` which return values that theoretically could be manipulated during the execution. + +## Vulnerable Scenario +[test scenarios](../../tests/balancer/readonly_reentrancy_test.sol) + +## Related Attacks +* [Sentimentxyz Exploit](https://quillaudits.medium.com/decoding-sentiment-protocols-1-million-exploit-quillaudits-f36bee77d376) +* [Sturdy Exploit](https://blog.solidityscan.com/sturdy-finance-hack-analysis-bd8605cd2956) + +## Recommendation + diff --git a/slither_pess/detectors/balancer/readonly_reentrancy.py b/slither_pess/detectors/balancer/readonly_reentrancy.py new file mode 100644 index 0000000..e69de29 diff --git a/tests/balancer/readonly_reentrancy_test.sol b/tests/balancer/readonly_reentrancy_test.sol new file mode 100644 index 0000000..e69de29 From 63f413a85da22e6d45b739028abd3ecd69295316 Mon Sep 17 00:00:00 2001 From: Yhtyyar Sahatov Date: Thu, 19 Oct 2023 14:55:11 +0300 Subject: [PATCH 02/48] working version --- package-lock.json | 48 +++++++++- package.json | 5 +- slither_pess/__init__.py | 8 +- .../__init__.py | 0 .../balancer_readonly_reentrancy.py | 96 +++++++++++++++++++ .../read_only_reentrancy.py | 0 .../reentrancy/__init__.py} | 0 .../reentrancy/reentrancy.py | 0 test_balancer.bash | 1 + tests/balancer/readonly_reentrancy_test.sol | 60 ++++++++++++ 10 files changed, 215 insertions(+), 3 deletions(-) rename slither_pess/detectors/{reentrancy => readonly_reentrancy}/__init__.py (100%) create mode 100644 slither_pess/detectors/readonly_reentrancy/balancer_readonly_reentrancy.py rename slither_pess/detectors/{ => readonly_reentrancy}/read_only_reentrancy.py (100%) rename slither_pess/detectors/{balancer/readonly_reentrancy.py => readonly_reentrancy/reentrancy/__init__.py} (100%) rename slither_pess/detectors/{ => readonly_reentrancy}/reentrancy/reentrancy.py (100%) create mode 100755 test_balancer.bash diff --git a/package-lock.json b/package-lock.json index cc36f30..dfcf7dc 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,13 +1,37 @@ { - "name": "slitherin", + "name": "custom_detectors", "lockfileVersion": 2, "requires": true, "packages": { "": { "dependencies": { + "@balancer-labs/v2-interfaces": "^0.4.0", + "@balancer-labs/v2-pool-utils": "^4.0.0", "@openzeppelin/contracts": "^4.9.3" } }, + "node_modules/@balancer-labs/v2-interfaces": { + "version": "0.4.0", + "resolved": "https://registry.npmjs.org/@balancer-labs/v2-interfaces/-/v2-interfaces-0.4.0.tgz", + "integrity": "sha512-K0ij26m8/UOvdPmrAnuh/C7kT8OQupsgV8KRyIt+aTHW1KbPOi4v8zLMwW2AwSYMSRjPK2A/ttlnNizT0iA4Qg==" + }, + "node_modules/@balancer-labs/v2-pool-utils": { + "version": "4.1.1", + "resolved": "https://registry.npmjs.org/@balancer-labs/v2-pool-utils/-/v2-pool-utils-4.1.1.tgz", + "integrity": "sha512-CZRXqD71Sog7zZh8ycdAN1Q0RpYPTvH7orQcWUNbj9ooWc5M7eqmgntsK8Me1a5AKroxf6rH0xLQBYoBaapbMA==", + "dependencies": { + "@balancer-labs/v2-interfaces": "0.4.0", + "@balancer-labs/v2-solidity-utils": "4.0.0" + } + }, + "node_modules/@balancer-labs/v2-solidity-utils": { + "version": "4.0.0", + "resolved": "https://registry.npmjs.org/@balancer-labs/v2-solidity-utils/-/v2-solidity-utils-4.0.0.tgz", + "integrity": "sha512-BB9bawgywV8q8v/m3xhYVI5X0rJ3ox64t4IYiAQvgdrR7+Sgn6AAkAP++odjbDJdCcM5wQMi92zwt50rq2nn6A==", + "dependencies": { + "@balancer-labs/v2-interfaces": "0.4.0" + } + }, "node_modules/@openzeppelin/contracts": { "version": "4.9.3", "resolved": "https://registry.npmjs.org/@openzeppelin/contracts/-/contracts-4.9.3.tgz", @@ -15,6 +39,28 @@ } }, "dependencies": { + "@balancer-labs/v2-interfaces": { + "version": "0.4.0", + "resolved": "https://registry.npmjs.org/@balancer-labs/v2-interfaces/-/v2-interfaces-0.4.0.tgz", + "integrity": "sha512-K0ij26m8/UOvdPmrAnuh/C7kT8OQupsgV8KRyIt+aTHW1KbPOi4v8zLMwW2AwSYMSRjPK2A/ttlnNizT0iA4Qg==" + }, + "@balancer-labs/v2-pool-utils": { + "version": "4.1.1", + "resolved": "https://registry.npmjs.org/@balancer-labs/v2-pool-utils/-/v2-pool-utils-4.1.1.tgz", + "integrity": "sha512-CZRXqD71Sog7zZh8ycdAN1Q0RpYPTvH7orQcWUNbj9ooWc5M7eqmgntsK8Me1a5AKroxf6rH0xLQBYoBaapbMA==", + "requires": { + "@balancer-labs/v2-interfaces": "0.4.0", + "@balancer-labs/v2-solidity-utils": "4.0.0" + } + }, + "@balancer-labs/v2-solidity-utils": { + "version": "4.0.0", + "resolved": "https://registry.npmjs.org/@balancer-labs/v2-solidity-utils/-/v2-solidity-utils-4.0.0.tgz", + "integrity": "sha512-BB9bawgywV8q8v/m3xhYVI5X0rJ3ox64t4IYiAQvgdrR7+Sgn6AAkAP++odjbDJdCcM5wQMi92zwt50rq2nn6A==", + "requires": { + "@balancer-labs/v2-interfaces": "0.4.0" + } + }, "@openzeppelin/contracts": { "version": "4.9.3", "resolved": "https://registry.npmjs.org/@openzeppelin/contracts/-/contracts-4.9.3.tgz", diff --git a/package.json b/package.json index b6f9903..016662d 100644 --- a/package.json +++ b/package.json @@ -1,5 +1,8 @@ { "dependencies": { - "@openzeppelin/contracts": "^4.9.3" + "@openzeppelin/contracts": "^4.9.3", + "@balancer-labs/v2-interfaces": "^0.4.0", + "@balancer-labs/v2-pool-utils": "^4.0.0" + } } diff --git a/slither_pess/__init__.py b/slither_pess/__init__.py index 6d18886..f54d75d 100644 --- a/slither_pess/__init__.py +++ b/slither_pess/__init__.py @@ -14,7 +14,9 @@ from slither_pess.detectors.timelock_controller import TimelockController from slither_pess.detectors.tx_gasprice_warning import TxGaspriceWarning from slither_pess.detectors.unprotected_initialize import UnprotectedInitialize -from slither_pess.detectors.read_only_reentrancy import ReadOnlyReentrancy +from slither_pess.detectors.readonly_reentrancy.read_only_reentrancy import ( + ReadOnlyReentrancy, +) from slither_pess.detectors.event_setter import EventSetter from slither_pess.detectors.before_token_transfer import BeforeTokenTransfer from slither_pess.detectors.uni_v2 import UniswapV2 @@ -22,6 +24,9 @@ from slither_pess.detectors.for_continue_increment import ForContinueIncrement from slither_pess.detectors.ecrecover import Ecrecover from slither_pess.detectors.public_vs_external import PublicVsExternal +from slither_pess.detectors.readonly_reentrancy.balancer_readonly_reentrancy import ( + BalancerReadonlyReentrancy, +) def make_plugin(): @@ -48,6 +53,7 @@ def make_plugin(): ArbitraryCall, Ecrecover, PublicVsExternal, + BalancerReadonlyReentrancy, ] plugin_printers = [] diff --git a/slither_pess/detectors/reentrancy/__init__.py b/slither_pess/detectors/readonly_reentrancy/__init__.py similarity index 100% rename from slither_pess/detectors/reentrancy/__init__.py rename to slither_pess/detectors/readonly_reentrancy/__init__.py diff --git a/slither_pess/detectors/readonly_reentrancy/balancer_readonly_reentrancy.py b/slither_pess/detectors/readonly_reentrancy/balancer_readonly_reentrancy.py new file mode 100644 index 0000000..97f548e --- /dev/null +++ b/slither_pess/detectors/readonly_reentrancy/balancer_readonly_reentrancy.py @@ -0,0 +1,96 @@ +from typing import List +from slither.utils.output import Output +from slither.detectors.abstract_detector import AbstractDetector, DetectorClassification +from slither.core.declarations import Function, Contract +from slither.core.cfg.node import Node + + +class BalancerReadonlyReentrancy(AbstractDetector): + """ + Sees if a contract has a beforeTokenTransfer function. + """ + + ARGUMENT = "pess-balancer-readonly-reentrancy" # slither will launch the detector with slither.py --detect mydetector + HELP = "beforeTokenTransfer function does not follow OZ documentation" + IMPACT = DetectorClassification.LOW + CONFIDENCE = DetectorClassification.HIGH + + WIKI = ( + "https://docs.openzeppelin.com/contracts/4.x/extending-contracts#rules_of_hooks" + ) + WIKI_TITLE = "Before Token Transfer" + WIKI_DESCRIPTION = "Follow OZ documentation using their contracts" + WIKI_EXPLOIT_SCENARIO = "-" + WIKI_RECOMMENDATION = ( + "Make sure that beforeTokenTransfer function is used in the correct way." + ) + + VULNERABLE_FUNCTION_CALLS = ["getRate", "getPoolTokens"] + visited = [] + contains_reentrancy_check = {} + + def is_balancer_integration(self, c: Contract) -> bool: + """ + Iterates over all external function calls, and checks the interface/contract name + for a specific keywords to decide if the contract integrates with balancer + """ + for ( + fcontract, + _, + ) in c.all_high_level_calls: + contract_name = fcontract.name.lower() + if any(map(lambda x: x in contract_name, ["balancer", "ivault", "pool"])): + return True + + def _has_reentrancy_check(self, node: Node) -> bool: + if node in self.visited: + return self.contains_reentrancy_check[node] + + self.visited.append(node) + self.contains_reentrancy_check[node] = False + + for c, n in node.high_level_calls: + if isinstance(n, Function): + if ( + n.name == "ensureNotInVaultContext" + and c.name == "VaultReentrancyLib" + ) or ( + n.name == "manageUserBalance" + ): # TODO check if errors out + self.contains_reentrancy_check[node] = True + return True + + has_check = False + for internal_call in node.internal_calls: + if isinstance(internal_call, Function): + has_check |= self._has_reentrancy_check(internal_call) + # self.contains_reentrancy_check[internal_call] |= has_check + + self.contains_reentrancy_check[node] = has_check + return has_check + + def _check_function(self, function: Function) -> list: + has_dangerous_call = False + dangerous_call = None + for n in function.nodes: + for c, fc in n.high_level_calls: + if isinstance(fc, Function): + if fc.name in self.VULNERABLE_FUNCTION_CALLS: + dangerous_call = fc + has_dangerous_call = True + break + + if has_dangerous_call and not any( + [self._has_reentrancy_check(node) for node in function.nodes] + ): + print("READONLY_REENTRANCY!!!") + + def _detect(self) -> List[Output]: + """Main function""" + res = [] + for contract in self.compilation_unit.contracts_derived: + if not self.is_balancer_integration(contract): + continue + for f in contract.functions_and_modifiers_declared: + self._check_function(f) + return res diff --git a/slither_pess/detectors/read_only_reentrancy.py b/slither_pess/detectors/readonly_reentrancy/read_only_reentrancy.py similarity index 100% rename from slither_pess/detectors/read_only_reentrancy.py rename to slither_pess/detectors/readonly_reentrancy/read_only_reentrancy.py diff --git a/slither_pess/detectors/balancer/readonly_reentrancy.py b/slither_pess/detectors/readonly_reentrancy/reentrancy/__init__.py similarity index 100% rename from slither_pess/detectors/balancer/readonly_reentrancy.py rename to slither_pess/detectors/readonly_reentrancy/reentrancy/__init__.py diff --git a/slither_pess/detectors/reentrancy/reentrancy.py b/slither_pess/detectors/readonly_reentrancy/reentrancy/reentrancy.py similarity index 100% rename from slither_pess/detectors/reentrancy/reentrancy.py rename to slither_pess/detectors/readonly_reentrancy/reentrancy/reentrancy.py diff --git a/test_balancer.bash b/test_balancer.bash new file mode 100755 index 0000000..2a15042 --- /dev/null +++ b/test_balancer.bash @@ -0,0 +1 @@ +slither tests/balancer/readonly_reentrancy_test.sol --detect pess-balancer-readonly-reentrancy --solc-remaps @=node_modules/@ \ No newline at end of file diff --git a/tests/balancer/readonly_reentrancy_test.sol b/tests/balancer/readonly_reentrancy_test.sol index e69de29..77499fb 100644 --- a/tests/balancer/readonly_reentrancy_test.sol +++ b/tests/balancer/readonly_reentrancy_test.sol @@ -0,0 +1,60 @@ +// SPDX-License-Identifier: MIT +pragma solidity ^0.8.18; +import {VaultReentrancyLib} from "@balancer-labs/v2-pool-utils/contracts/lib/VaultReentrancyLib.sol"; +import "@balancer-labs/v2-interfaces/contracts/vault/IVault.sol"; + +interface IBalancerPool { + function getRate() external view returns (uint); +} + +contract BalancerIntegration { + address balancerVault; + + constructor() {} + + function getPriceVulnerable(address vault) public returns (uint) { + return IBalancerPool(vault).getRate(); + } + + function getPriceVulnerable2(address vault) public { + bytes32 poolId = "0x123"; + uint256[] memory balances = new uint256[](10); + (, balances, ) = IVault(balancerVault).getPoolTokens(poolId); + } + + function _ensureNotReentrant() internal { + VaultReentrancyLib.ensureNotInVaultContext(IVault(balancerVault)); + } + + function _ensureNotReentrant2() internal { + IVault(balancerVault).manageUserBalance(new IVault.UserBalanceOp[](0)); + } + + function getPriceOk(address vault) public returns (uint) { + VaultReentrancyLib.ensureNotInVaultContext(IVault(balancerVault)); + return IBalancerPool(vault).getRate(); + } + + function getPriceOk2(address vault) public returns (uint) { + _ensureNotReentrant(); + return IBalancerPool(vault).getRate(); + } + + function getPriceOk3(address vault) public returns (uint) { + _ensureNotReentrant2(); + return IBalancerPool(vault).getRate(); + } + + function getPriceOk4(address vault) public returns (uint) { + uint a = IBalancerPool(vault).getRate(); + _ensureNotReentrant2(); + return a; + } + + function getPriceOk5(address vault) public { + VaultReentrancyLib.ensureNotInVaultContext(IVault(balancerVault)); + bytes32 poolId = "0x123"; + uint256[] memory balances = new uint256[](10); + (, balances, ) = IVault(balancerVault).getPoolTokens(poolId); + } +} From 3bd7b0447517fa5a84056968628f0ab901d01b39 Mon Sep 17 00:00:00 2001 From: Yhtyyar Sahatov Date: Thu, 19 Oct 2023 15:19:43 +0300 Subject: [PATCH 03/48] updated severity --- .../balancer_readonly_reentrancy.py | 36 +++++++++++++++---- 1 file changed, 29 insertions(+), 7 deletions(-) diff --git a/slither_pess/detectors/readonly_reentrancy/balancer_readonly_reentrancy.py b/slither_pess/detectors/readonly_reentrancy/balancer_readonly_reentrancy.py index 97f548e..f88f8d5 100644 --- a/slither_pess/detectors/readonly_reentrancy/balancer_readonly_reentrancy.py +++ b/slither_pess/detectors/readonly_reentrancy/balancer_readonly_reentrancy.py @@ -11,8 +11,8 @@ class BalancerReadonlyReentrancy(AbstractDetector): """ ARGUMENT = "pess-balancer-readonly-reentrancy" # slither will launch the detector with slither.py --detect mydetector - HELP = "beforeTokenTransfer function does not follow OZ documentation" - IMPACT = DetectorClassification.LOW + HELP = "Balancer readonly-reentrancy" + IMPACT = DetectorClassification.HIGH CONFIDENCE = DetectorClassification.HIGH WIKI = ( @@ -51,6 +51,8 @@ def _has_reentrancy_check(self, node: Node) -> bool: for c, n in node.high_level_calls: if isinstance(n, Function): + if not n.name: + continue if ( n.name == "ensureNotInVaultContext" and c.name == "VaultReentrancyLib" @@ -76,21 +78,41 @@ def _check_function(self, function: Function) -> list: for c, fc in n.high_level_calls: if isinstance(fc, Function): if fc.name in self.VULNERABLE_FUNCTION_CALLS: - dangerous_call = fc + dangerous_call = n # Saving only first dangerous call has_dangerous_call = True break if has_dangerous_call and not any( [self._has_reentrancy_check(node) for node in function.nodes] ): - print("READONLY_REENTRANCY!!!") + return [dangerous_call] + return [] def _detect(self) -> List[Output]: """Main function""" - res = [] + result = [] for contract in self.compilation_unit.contracts_derived: if not self.is_balancer_integration(contract): continue + res = [] for f in contract.functions_and_modifiers_declared: - self._check_function(f) - return res + function_result = self._check_function(f) + if function_result: + res.extend(function_result) + if res: + info = [ + "Balancer readonly-reentrancy in vulnerability detected in ", + contract, + ":\n", + ] + for r in res: + info += [ + "\tThe answer of ", + r, + " call could be manipulated through readonly-reentrancy\n", + ] + res = self.generate_result(info) + res.add(r) + result.append(res) + + return result From e33037aac77a40aeb875fc5628510dbcfd8bb301 Mon Sep 17 00:00:00 2001 From: Yhtyyar Sahatov Date: Thu, 19 Oct 2023 16:04:30 +0300 Subject: [PATCH 04/48] fixed docs --- docs/balancer/readonly_reentrancy.md | 17 +++++++++++------ .../balancer_readonly_reentrancy.py | 14 +++++--------- 2 files changed, 16 insertions(+), 15 deletions(-) diff --git a/docs/balancer/readonly_reentrancy.md b/docs/balancer/readonly_reentrancy.md index 4689a1c..7094fb8 100644 --- a/docs/balancer/readonly_reentrancy.md +++ b/docs/balancer/readonly_reentrancy.md @@ -1,19 +1,24 @@ # Balancer Readonly Reentrancy ## Configuration -* Check: `bal-readonly-reentrancy` -* Severity: `High` -* Confidence: `Medium` + +- Check: `pess-balancer-readonly-reentrancy` +- Severity: `High` +- Confidence: `Medium` ## Description -Highlights the use of Balancer getter functions `getRate` and `getPoolTokens` which return values that theoretically could be manipulated during the execution. + +Highlights the use of Balancer getter functions `getRate` and `getPoolTokens` (which are not checked for readonly reentrancy via `VaultReentrancyLib.ensureNotInVaultContext` or `IVault.manageUserBalance`), which return values that theoretically could be manipulated during the execution. ## Vulnerable Scenario + [test scenarios](../../tests/balancer/readonly_reentrancy_test.sol) ## Related Attacks -* [Sentimentxyz Exploit](https://quillaudits.medium.com/decoding-sentiment-protocols-1-million-exploit-quillaudits-f36bee77d376) -* [Sturdy Exploit](https://blog.solidityscan.com/sturdy-finance-hack-analysis-bd8605cd2956) + +- [Sentimentxyz Exploit](https://quillaudits.medium.com/decoding-sentiment-protocols-1-million-exploit-quillaudits-f36bee77d376) +- [Sturdy Exploit](https://blog.solidityscan.com/sturdy-finance-hack-analysis-bd8605cd2956) ## Recommendation +- [Official Balancer recomendation](https://docs.balancer.fi/concepts/advanced/valuing-bpt/valuing-bpt.html#on-chain-price-evaluation) diff --git a/slither_pess/detectors/readonly_reentrancy/balancer_readonly_reentrancy.py b/slither_pess/detectors/readonly_reentrancy/balancer_readonly_reentrancy.py index f88f8d5..70eca69 100644 --- a/slither_pess/detectors/readonly_reentrancy/balancer_readonly_reentrancy.py +++ b/slither_pess/detectors/readonly_reentrancy/balancer_readonly_reentrancy.py @@ -13,17 +13,13 @@ class BalancerReadonlyReentrancy(AbstractDetector): ARGUMENT = "pess-balancer-readonly-reentrancy" # slither will launch the detector with slither.py --detect mydetector HELP = "Balancer readonly-reentrancy" IMPACT = DetectorClassification.HIGH - CONFIDENCE = DetectorClassification.HIGH + CONFIDENCE = DetectorClassification.MEDIUM - WIKI = ( - "https://docs.openzeppelin.com/contracts/4.x/extending-contracts#rules_of_hooks" - ) - WIKI_TITLE = "Before Token Transfer" - WIKI_DESCRIPTION = "Follow OZ documentation using their contracts" + WIKI = "https://github.com/pessimistic-io/slitherin/blob/master/docs/balancer/readonly_reentrancy.md" + WIKI_TITLE = "Balancer Readonly Reentrancy" + WIKI_DESCRIPTION = "Check docs" WIKI_EXPLOIT_SCENARIO = "-" - WIKI_RECOMMENDATION = ( - "Make sure that beforeTokenTransfer function is used in the correct way." - ) + WIKI_RECOMMENDATION = "Check docs" VULNERABLE_FUNCTION_CALLS = ["getRate", "getPoolTokens"] visited = [] From e3a0cf9bbde6137746ebba101e487ba202107c3f Mon Sep 17 00:00:00 2001 From: Nikita Kirillov Date: Thu, 19 Oct 2023 18:11:34 +0500 Subject: [PATCH 05/48] output_result_typo --- .../readonly_reentrancy/balancer_readonly_reentrancy.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/slither_pess/detectors/readonly_reentrancy/balancer_readonly_reentrancy.py b/slither_pess/detectors/readonly_reentrancy/balancer_readonly_reentrancy.py index 70eca69..4dba564 100644 --- a/slither_pess/detectors/readonly_reentrancy/balancer_readonly_reentrancy.py +++ b/slither_pess/detectors/readonly_reentrancy/balancer_readonly_reentrancy.py @@ -97,7 +97,7 @@ def _detect(self) -> List[Output]: res.extend(function_result) if res: info = [ - "Balancer readonly-reentrancy in vulnerability detected in ", + "Balancer readonly-reentrancy vulnerability detected in ", contract, ":\n", ] From 13ef10d165e820b5a75069e7ed4c5800ee511396 Mon Sep 17 00:00:00 2001 From: nikolay19 Date: Mon, 12 Feb 2024 12:17:15 +0100 Subject: [PATCH 06/48] bench updates&workflow google sheet upload --- .github/workflows/benchmark.yml | 9 ++++++++- slitherin-benchmark | 2 +- 2 files changed, 9 insertions(+), 2 deletions(-) diff --git a/.github/workflows/benchmark.yml b/.github/workflows/benchmark.yml index 66c4349..89b101d 100644 --- a/.github/workflows/benchmark.yml +++ b/.github/workflows/benchmark.yml @@ -44,7 +44,14 @@ jobs: - name: Run Benchmark run: | cd slitherin-benchmark/ - python runner.py -i contracts/mainnet -o mainnet.csv --limit 7000 + python runner.py -i contracts/mainnet -o mainnet.csv --limit 10000 --skip-duplicates --skip-libs + - name: Upload sheet + run: | + echo $GOOGLE_JWT > service_account.json + python save_sheet.py -i mainnet.csv -sa service_account.json -si $GOOGLE_SHEET_ID -ln mainnet + env: + GOOGLE_JWT : ${{secrets.SERVICE_ACCOUNT}} + GOOGLE_SHEET_ID : ${{ secrets.GOOGLE_SHEET_ID }} - name: 'Upload Artifact' uses: actions/upload-artifact@v3 with: diff --git a/slitherin-benchmark b/slitherin-benchmark index 12792f9..0a6fe9b 160000 --- a/slitherin-benchmark +++ b/slitherin-benchmark @@ -1 +1 @@ -Subproject commit 12792f91792dfe5a90ed9a0fc955606082b92305 +Subproject commit 0a6fe9bf77bd5f56594c136f1845b84533d603a1 From 67a88c55ddd69f77a607a083366242d6692ac367 Mon Sep 17 00:00:00 2001 From: Nikolay Date: Mon, 12 Feb 2024 22:06:52 +0300 Subject: [PATCH 07/48] bench 8000 contracts limit --- .github/workflows/benchmark.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/benchmark.yml b/.github/workflows/benchmark.yml index 89b101d..79165a1 100644 --- a/.github/workflows/benchmark.yml +++ b/.github/workflows/benchmark.yml @@ -44,7 +44,7 @@ jobs: - name: Run Benchmark run: | cd slitherin-benchmark/ - python runner.py -i contracts/mainnet -o mainnet.csv --limit 10000 --skip-duplicates --skip-libs + python runner.py -i contracts/mainnet -o mainnet.csv --limit 8000 --skip-duplicates --skip-libs - name: Upload sheet run: | echo $GOOGLE_JWT > service_account.json From 417e8665fd01b9af9817daaba7868af5ae2cb344 Mon Sep 17 00:00:00 2001 From: Nikolay Date: Tue, 13 Feb 2024 09:24:20 +0300 Subject: [PATCH 08/48] Update benchmark.yml --- .github/workflows/benchmark.yml | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/.github/workflows/benchmark.yml b/.github/workflows/benchmark.yml index 79165a1..b9f4cec 100644 --- a/.github/workflows/benchmark.yml +++ b/.github/workflows/benchmark.yml @@ -47,6 +47,7 @@ jobs: python runner.py -i contracts/mainnet -o mainnet.csv --limit 8000 --skip-duplicates --skip-libs - name: Upload sheet run: | + cd slitherin-benchmark/ echo $GOOGLE_JWT > service_account.json python save_sheet.py -i mainnet.csv -sa service_account.json -si $GOOGLE_SHEET_ID -ln mainnet env: @@ -96,6 +97,14 @@ jobs: run: | cd slitherin-benchmark/ python runner.py -i contracts/openzeppelin -o oz.csv -eo oz_extra.csv + - name: Upload sheet + run: | + cd slitherin-benchmark/ + echo $GOOGLE_JWT > service_account.json + python save_sheet.py -i oz.csv -sa service_account.json -si $GOOGLE_SHEET_ID -ln OZ + env: + GOOGLE_JWT : ${{secrets.SERVICE_ACCOUNT}} + GOOGLE_SHEET_ID : ${{ secrets.GOOGLE_SHEET_ID }} - name: 'Upload Artifact' uses: actions/upload-artifact@v3 with: From 98921ea3a91d3b21b82fe916f0bb72ac9446faa8 Mon Sep 17 00:00:00 2001 From: nikolay19 Date: Thu, 15 Feb 2024 22:03:30 +0300 Subject: [PATCH 09/48] workflow for old version bench --- .github/workflows/old_version.yml | 117 ++++++++++++++++++++++++++++++ slitherin-benchmark | 2 +- 2 files changed, 118 insertions(+), 1 deletion(-) create mode 100644 .github/workflows/old_version.yml diff --git a/.github/workflows/old_version.yml b/.github/workflows/old_version.yml new file mode 100644 index 0000000..b219aa3 --- /dev/null +++ b/.github/workflows/old_version.yml @@ -0,0 +1,117 @@ +name: Run Benchmark + +on: + workflow_dispatch: # Ручной запуск через UI Гитхаба +jobs: + RunBenchmarkOld: + runs-on: ubuntu-latest + env: + slitherin_version: 0.1.0 + steps: + - name: Checkout repository + uses: actions/checkout@v4 + with: + submodules: 'true' + - name: Set up Python + uses: actions/setup-python@v4 + with: + python-version: '3.x' + - name: Set up Node + uses: actions/setup-node@v4 + with: + node-version: '18.x' + - name: Update pip + run: python -m pip install --upgrade pip + - name: Install solc-select + run: python -m pip install solc-select + - name: Install Slither + run: python -m pip install slither-analyzer + - name: Install Setuptools + run: python -m pip install setuptools + - name: Install Slitherin + run: python -m pip install slitherin==$slitherin_version + - name: Configure + run: | + cd slitherin-benchmark/ + mv example.config.py config.py + - name: Install benchmark requirements + run: | + cd slitherin-benchmark/ + python -m pip install -r requirements.txt + - name: Run Benchmark + run: | + cd slitherin-benchmark/ + python runner.py -i contracts/mainnet -o mainnet.csv --limit 8000 --skip-duplicates --skip-libs --use-slither + - name: Upload sheet + run: | + cd slitherin-benchmark/ + echo $GOOGLE_JWT > service_account.json + python save_sheet.py -i mainnet.csv -sa service_account.json -si $GOOGLE_SHEET_ID -ln mainnet -sv "slitherin $slitherin_version" + env: + GOOGLE_JWT : ${{secrets.SERVICE_ACCOUNT}} + GOOGLE_SHEET_ID : ${{ secrets.GOOGLE_SHEET_ID }} + - name: 'Upload Artifact' + uses: actions/upload-artifact@v3 + with: + name: mainnet + path: slitherin-benchmark/mainnet.csv + RunBenchmarkOZOld: + runs-on: ubuntu-latest + env: + slitherin_version: 0.1.0 + steps: + - name: Checkout repository + uses: actions/checkout@v4 + with: + submodules: 'true' + - name: Set up Python + uses: actions/setup-python@v4 + with: + python-version: '3.x' + - name: Set up Node + uses: actions/setup-node@v4 + with: + node-version: '18.x' + - name: Update pip + run: python -m pip install --upgrade pip + - name: Install solc-select + run: python -m pip install solc-select + - name: Install Slither + run: python -m pip install slither-analyzer + - name: Install Setuptools + run: python -m pip install setuptools + - name: Install Slitherin + run: python -m pip install slitherin==$slitherin_version + - name: Configure + run: | + cd slitherin-benchmark/ + mv example.config.py config.py + - name: Install node dependencies + run: npm ci + - name: Install benchmark requirements + run: | + cd slitherin-benchmark/ + python -m pip install -r requirements.txt + - name: Run Benchmark + run: | + cd slitherin-benchmark/ + python runner.py -i contracts/openzeppelin -o oz.csv -eo oz_extra.csv --use-slither + - name: Upload sheet + run: | + cd slitherin-benchmark/ + echo $GOOGLE_JWT > service_account.json + python save_sheet.py -i oz.csv -sa service_account.json -si $GOOGLE_SHEET_ID -ln OZ -sv "slitherin $slitherin_version" + env: + GOOGLE_JWT : ${{secrets.SERVICE_ACCOUNT}} + GOOGLE_SHEET_ID : ${{ secrets.GOOGLE_SHEET_ID }} + - name: 'Upload Artifact' + uses: actions/upload-artifact@v3 + with: + name: oz + path: slitherin-benchmark/oz.csv + - name: 'Upload Artifact' + uses: actions/upload-artifact@v3 + with: + name: oz_extra + path: slitherin-benchmark/oz_extra.csv + diff --git a/slitherin-benchmark b/slitherin-benchmark index 0a6fe9b..12792f9 160000 --- a/slitherin-benchmark +++ b/slitherin-benchmark @@ -1 +1 @@ -Subproject commit 0a6fe9bf77bd5f56594c136f1845b84533d603a1 +Subproject commit 12792f91792dfe5a90ed9a0fc955606082b92305 From 94218f158ef76ffeef1b9404e70f28056de604b2 Mon Sep 17 00:00:00 2001 From: Nikolay Date: Thu, 15 Feb 2024 22:38:50 +0300 Subject: [PATCH 10/48] Update old_version.yml --- .github/workflows/old_version.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/.github/workflows/old_version.yml b/.github/workflows/old_version.yml index b219aa3..758dbc8 100644 --- a/.github/workflows/old_version.yml +++ b/.github/workflows/old_version.yml @@ -100,6 +100,7 @@ jobs: run: | cd slitherin-benchmark/ echo $GOOGLE_JWT > service_account.json + ls python save_sheet.py -i oz.csv -sa service_account.json -si $GOOGLE_SHEET_ID -ln OZ -sv "slitherin $slitherin_version" env: GOOGLE_JWT : ${{secrets.SERVICE_ACCOUNT}} From 53ef547048bd244a3d646c087fd34ca9aec3a652 Mon Sep 17 00:00:00 2001 From: nikolay19 Date: Thu, 15 Feb 2024 23:13:46 +0300 Subject: [PATCH 11/48] bench updates --- slitherin-benchmark | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/slitherin-benchmark b/slitherin-benchmark index 12792f9..96f43d6 160000 --- a/slitherin-benchmark +++ b/slitherin-benchmark @@ -1 +1 @@ -Subproject commit 12792f91792dfe5a90ed9a0fc955606082b92305 +Subproject commit 96f43d6b35076a2b2912d9dfbc9695de0c8dfede From dc6210dd3831febbf8e436cebb4bc37156924d62 Mon Sep 17 00:00:00 2001 From: Nikolay Date: Thu, 15 Feb 2024 23:25:13 +0300 Subject: [PATCH 12/48] Update benchmark.yml --- .github/workflows/benchmark.yml | 20 ++++++++++++++++++-- 1 file changed, 18 insertions(+), 2 deletions(-) diff --git a/.github/workflows/benchmark.yml b/.github/workflows/benchmark.yml index 66c4349..48cebf0 100644 --- a/.github/workflows/benchmark.yml +++ b/.github/workflows/benchmark.yml @@ -34,6 +34,7 @@ jobs: - name: Configure run: | cd slitherin-benchmark/ + ls mv example.config.py config.py - name: Install node dependencies run: npm ci @@ -44,7 +45,15 @@ jobs: - name: Run Benchmark run: | cd slitherin-benchmark/ - python runner.py -i contracts/mainnet -o mainnet.csv --limit 7000 + python runner.py -i contracts/mainnet -o mainnet.csv --limit 8000 --skip-duplicates --skip-libs + - name: Upload sheet + run: | + cd slitherin-benchmark/ + echo $GOOGLE_JWT > service_account.json + python save_sheet.py -i mainnet.csv -sa service_account.json -si $GOOGLE_SHEET_ID -ln mainnet + env: + GOOGLE_JWT : ${{secrets.SERVICE_ACCOUNT}} + GOOGLE_SHEET_ID : ${{ secrets.GOOGLE_SHEET_ID }} - name: 'Upload Artifact' uses: actions/upload-artifact@v3 with: @@ -89,6 +98,14 @@ jobs: run: | cd slitherin-benchmark/ python runner.py -i contracts/openzeppelin -o oz.csv -eo oz_extra.csv + - name: Upload sheet + run: | + cd slitherin-benchmark/ + echo $GOOGLE_JWT > service_account.json + python save_sheet.py -i oz.csv -sa service_account.json -si $GOOGLE_SHEET_ID -ln OZ + env: + GOOGLE_JWT : ${{secrets.SERVICE_ACCOUNT}} + GOOGLE_SHEET_ID : ${{ secrets.GOOGLE_SHEET_ID }} - name: 'Upload Artifact' uses: actions/upload-artifact@v3 with: @@ -99,4 +116,3 @@ jobs: with: name: oz_extra path: slitherin-benchmark/oz_extra.csv - From 32b4e0c29049ada7c61a22ffa4f81235259ac57b Mon Sep 17 00:00:00 2001 From: Nikolay Date: Mon, 19 Feb 2024 21:51:28 +0300 Subject: [PATCH 13/48] Create benchmark_old.yml --- .github/workflows/benchmark_old.yml | 117 ++++++++++++++++++++++++++++ 1 file changed, 117 insertions(+) create mode 100644 .github/workflows/benchmark_old.yml diff --git a/.github/workflows/benchmark_old.yml b/.github/workflows/benchmark_old.yml new file mode 100644 index 0000000..789d291 --- /dev/null +++ b/.github/workflows/benchmark_old.yml @@ -0,0 +1,117 @@ +name: Run Benchmark + +on: + workflow_dispatch: # Ручной запуск через UI Гитхаба +jobs: + RunBenchmarkOld: + runs-on: ubuntu-latest + env: + slitherin_version: 0.1.0 + steps: + - name: Checkout repository + uses: actions/checkout@v4 + with: + submodules: 'true' + - name: Set up Python + uses: actions/setup-python@v4 + with: + python-version: '3.x' + - name: Set up Node + uses: actions/setup-node@v4 + with: + node-version: '18.x' + - name: Update pip + run: python -m pip install --upgrade pip + - name: Install solc-select + run: python -m pip install solc-select + - name: Install Slither + run: python -m pip install slither-analyzer + - name: Install Setuptools + run: python -m pip install setuptools + - name: Install Slitherin + run: python -m pip install slitherin==$slitherin_version + - name: Configure + run: | + cd slitherin-benchmark/ + mv example.config.py config.py + - name: Install benchmark requirements + run: | + cd slitherin-benchmark/ + python -m pip install -r requirements.txt + - name: Run Benchmark + run: | + cd slitherin-benchmark/ + python runner.py -i contracts/mainnet -o mainnet.csv --limit 8000 --skip-duplicates --skip-libs --use-slither + - name: Upload sheet + run: | + cd slitherin-benchmark/ + echo $GOOGLE_JWT > service_account.json + python save_sheet.py -i mainnet.csv -sa service_account.json -si $GOOGLE_SHEET_ID -ln mainnet -sv "slitherin $slitherin_version" + env: + GOOGLE_JWT : ${{secrets.SERVICE_ACCOUNT}} + GOOGLE_SHEET_ID : ${{ secrets.GOOGLE_SHEET_ID }} + - name: 'Upload Artifact' + uses: actions/upload-artifact@v3 + with: + name: mainnet + path: slitherin-benchmark/mainnet.csv + RunBenchmarkOZOld: + runs-on: ubuntu-latest + env: + slitherin_version: 0.1.0 + steps: + - name: Checkout repository + uses: actions/checkout@v4 + with: + submodules: 'true' + - name: Set up Python + uses: actions/setup-python@v4 + with: + python-version: '3.x' + - name: Set up Node + uses: actions/setup-node@v4 + with: + node-version: '18.x' + - name: Update pip + run: python -m pip install --upgrade pip + - name: Install solc-select + run: python -m pip install solc-select + - name: Install Slither + run: python -m pip install slither-analyzer + - name: Install Setuptools + run: python -m pip install setuptools + - name: Install Slitherin + run: python -m pip install slitherin==$slitherin_version + - name: Configure + run: | + cd slitherin-benchmark/ + mv example.config.py config.py + - name: Install node dependencies + run: npm ci + - name: Install benchmark requirements + run: | + cd slitherin-benchmark/ + python -m pip install -r requirements.txt + - name: Run Benchmark + run: | + cd slitherin-benchmark/ + python runner.py -i contracts/openzeppelin -o oz.csv -eo oz_extra.csv --use-slither + - name: Upload sheet + run: | + cd slitherin-benchmark/ + echo $GOOGLE_JWT > service_account.json + ls + python save_sheet.py -i oz.csv -sa service_account.json -si $GOOGLE_SHEET_ID -ln OZ -sv "slitherin $slitherin_version" + env: + GOOGLE_JWT : ${{secrets.SERVICE_ACCOUNT}} + GOOGLE_SHEET_ID : ${{ secrets.GOOGLE_SHEET_ID }} + - name: 'Upload Artifact' + uses: actions/upload-artifact@v3 + with: + name: oz + path: slitherin-benchmark/oz.csv + - name: 'Upload Artifact' + uses: actions/upload-artifact@v3 + with: + name: oz_extra + path: slitherin-benchmark/oz_extra.csv From e1cb898b515559c878e3d1817a703199af06a3c7 Mon Sep 17 00:00:00 2001 From: nikolay19 Date: Mon, 19 Feb 2024 22:26:54 +0300 Subject: [PATCH 14/48] remove action --- .github/workflows/benchmark_old.yml | 117 ---------------------------- 1 file changed, 117 deletions(-) delete mode 100644 .github/workflows/benchmark_old.yml diff --git a/.github/workflows/benchmark_old.yml b/.github/workflows/benchmark_old.yml deleted file mode 100644 index 789d291..0000000 --- a/.github/workflows/benchmark_old.yml +++ /dev/null @@ -1,117 +0,0 @@ -name: Run Benchmark - -on: - workflow_dispatch: # Ручной запуск через UI Гитхаба -jobs: - RunBenchmarkOld: - runs-on: ubuntu-latest - env: - slitherin_version: 0.1.0 - steps: - - name: Checkout repository - uses: actions/checkout@v4 - with: - submodules: 'true' - - name: Set up Python - uses: actions/setup-python@v4 - with: - python-version: '3.x' - - name: Set up Node - uses: actions/setup-node@v4 - with: - node-version: '18.x' - - name: Update pip - run: python -m pip install --upgrade pip - - name: Install solc-select - run: python -m pip install solc-select - - name: Install Slither - run: python -m pip install slither-analyzer - - name: Install Setuptools - run: python -m pip install setuptools - - name: Install Slitherin - run: python -m pip install slitherin==$slitherin_version - - name: Configure - run: | - cd slitherin-benchmark/ - mv example.config.py config.py - - name: Install benchmark requirements - run: | - cd slitherin-benchmark/ - python -m pip install -r requirements.txt - - name: Run Benchmark - run: | - cd slitherin-benchmark/ - python runner.py -i contracts/mainnet -o mainnet.csv --limit 8000 --skip-duplicates --skip-libs --use-slither - - name: Upload sheet - run: | - cd slitherin-benchmark/ - echo $GOOGLE_JWT > service_account.json - python save_sheet.py -i mainnet.csv -sa service_account.json -si $GOOGLE_SHEET_ID -ln mainnet -sv "slitherin $slitherin_version" - env: - GOOGLE_JWT : ${{secrets.SERVICE_ACCOUNT}} - GOOGLE_SHEET_ID : ${{ secrets.GOOGLE_SHEET_ID }} - - name: 'Upload Artifact' - uses: actions/upload-artifact@v3 - with: - name: mainnet - path: slitherin-benchmark/mainnet.csv - RunBenchmarkOZOld: - runs-on: ubuntu-latest - env: - slitherin_version: 0.1.0 - steps: - - name: Checkout repository - uses: actions/checkout@v4 - with: - submodules: 'true' - - name: Set up Python - uses: actions/setup-python@v4 - with: - python-version: '3.x' - - name: Set up Node - uses: actions/setup-node@v4 - with: - node-version: '18.x' - - name: Update pip - run: python -m pip install --upgrade pip - - name: Install solc-select - run: python -m pip install solc-select - - name: Install Slither - run: python -m pip install slither-analyzer - - name: Install Setuptools - run: python -m pip install setuptools - - name: Install Slitherin - run: python -m pip install slitherin==$slitherin_version - - name: Configure - run: | - cd slitherin-benchmark/ - mv example.config.py config.py - - name: Install node dependencies - run: npm ci - - name: Install benchmark requirements - run: | - cd slitherin-benchmark/ - python -m pip install -r requirements.txt - - name: Run Benchmark - run: | - cd slitherin-benchmark/ - python runner.py -i contracts/openzeppelin -o oz.csv -eo oz_extra.csv --use-slither - - name: Upload sheet - run: | - cd slitherin-benchmark/ - echo $GOOGLE_JWT > service_account.json - ls - python save_sheet.py -i oz.csv -sa service_account.json -si $GOOGLE_SHEET_ID -ln OZ -sv "slitherin $slitherin_version" - env: - GOOGLE_JWT : ${{secrets.SERVICE_ACCOUNT}} - GOOGLE_SHEET_ID : ${{ secrets.GOOGLE_SHEET_ID }} - - name: 'Upload Artifact' - uses: actions/upload-artifact@v3 - with: - name: oz - path: slitherin-benchmark/oz.csv - - name: 'Upload Artifact' - uses: actions/upload-artifact@v3 - with: - name: oz_extra - path: slitherin-benchmark/oz_extra.csv From 115cd26ea3ff8cf6bd7c476254e24562535d5eb1 Mon Sep 17 00:00:00 2001 From: Joran Honig Date: Thu, 22 Feb 2024 11:00:58 -0600 Subject: [PATCH 15/48] add napalm integration --- setup.py | 3 +-- slitherin/napalm.py | 12 ++++++++++++ 2 files changed, 13 insertions(+), 2 deletions(-) create mode 100644 slitherin/napalm.py diff --git a/setup.py b/setup.py index 66bfb56..c276088 100644 --- a/setup.py +++ b/setup.py @@ -20,16 +20,15 @@ "Topic :: Software Development :: Libraries", ], version=SLITHERIN_VERSION, - packages=find_packages(), license="AGPL-3.0", python_requires=">=3.8", - install_requires=["slither-analyzer>=0.10.0"], extras_requires={ "dev": ["twine>=4.0.2"], }, entry_points={ "slither_analyzer.plugin": "slither slitherin-plugins=slitherin:make_plugin", "console_scripts": ["slitherin=slitherin.cli:main"], + "napalm.collection": ["slitherin=slitherin.napalm:entry_point"], }, include_package_data=True, ) diff --git a/slitherin/napalm.py b/slitherin/napalm.py new file mode 100644 index 0000000..3e10b37 --- /dev/null +++ b/slitherin/napalm.py @@ -0,0 +1,12 @@ +import napalm.package +import slitherin.detectors + + +def entry_point(): + """This is the entry point for the napalm package. + + It provisions your detection modules and provides them to napalm. + + It returns a dictionary of Collections, keyed by the name of the collection. + """ + return napalm.package.entry_point(slitherin.detectors) From f4f0c5aef06383b2d841420527878bb048789e00 Mon Sep 17 00:00:00 2001 From: Yhtyyar Sahatov Date: Mon, 26 Feb 2024 14:32:09 +0300 Subject: [PATCH 16/48] removed arbitrum solidity version detector --- slitherin/__init__.py | 2 - .../detectors/arbitrum/solidity_version.py | 61 ------------------- tests/arbitrum_solidity_version_test.sol | 4 -- 3 files changed, 67 deletions(-) delete mode 100644 slitherin/detectors/arbitrum/solidity_version.py delete mode 100644 tests/arbitrum_solidity_version_test.sol diff --git a/slitherin/__init__.py b/slitherin/__init__.py index 35c1218..cd9e78b 100644 --- a/slitherin/__init__.py +++ b/slitherin/__init__.py @@ -26,7 +26,6 @@ from slitherin.detectors.arbitrum.arbitrum_prevrandao_difficulty import ( ArbitrumPrevrandaoDifficulty, ) -from slitherin.detectors.arbitrum.solidity_version import ArbitrumSolidityVersion from slitherin.detectors.arbitrum.block_number_timestamp import ( ArbitrumBlockNumberTimestamp, ) @@ -34,7 +33,6 @@ artbitrum_detectors = [ ArbitrumPrevrandaoDifficulty, - ArbitrumSolidityVersion, ArbitrumBlockNumberTimestamp, ] diff --git a/slitherin/detectors/arbitrum/solidity_version.py b/slitherin/detectors/arbitrum/solidity_version.py deleted file mode 100644 index 34a2def..0000000 --- a/slitherin/detectors/arbitrum/solidity_version.py +++ /dev/null @@ -1,61 +0,0 @@ -import os -from typing import List -from slither.utils.output import Output -from slither.core.cfg.node import Node -from slither.detectors.abstract_detector import ( - AbstractDetector, - DetectorClassification, - make_solc_versions, -) -from slither.core.declarations import Function -from slither.slithir.operations.event_call import EventCall -from slither.analyses.data_dependency.data_dependency import is_dependent -from slither.slithir.operations import SolidityCall -from slither.core.declarations.solidity_variables import ( - SolidityVariableComposed, - SolidityFunction, -) - -from ...consts import ARBITRUM_KEY - - -class ArbitrumSolidityVersion(AbstractDetector): - """ - Checks that sol version >= 0.8.20 is not used inside an Arbitrum contract - """ - - ARGUMENT = "pess-arb-solidity-version" # slither will launch the detector with slither.py --detect mydetector - HELP = "sol version >= 0.8.20 is used in contract that will be deployed to Arbitrum (Potential usage of PUSH0, which is not supported)" - IMPACT = DetectorClassification.HIGH - CONFIDENCE = DetectorClassification.LOW - - WIKI = "https://github.com/pessimistic-io/slitherin/blob/master/docs/arb_difficulty_randao.md" - WIKI_TITLE = "Arbitrum solidity version" - WIKI_DESCRIPTION = "Potential usage of PUSH0 opcode" - WIKI_EXPLOIT_SCENARIO = "N/A" - WIKI_RECOMMENDATION = ( - "Either, use versions 0.8.19 and below, or EVM versions below shanghai" - ) - - def _detect(self) -> List[Output]: - """Main function""" - results = [] - - if os.getenv(ARBITRUM_KEY) is None: - return results - - if ( - self.compilation_unit.is_solidity - and self.compilation_unit.solc_version in make_solc_versions(8, 20, 99) - ): - results.append( - self.generate_result( - [ - "Potential usage of solidity version >= 0.8.20 detected. Solidity in these versions will utilize PUSH0 opcode, ", - "which is not supported on Arbitrum. ", - "Either, use versions 0.8.19 and below, or EVM versions below shanghai", - ] - ) - ) - - return results diff --git a/tests/arbitrum_solidity_version_test.sol b/tests/arbitrum_solidity_version_test.sol deleted file mode 100644 index fbe7da6..0000000 --- a/tests/arbitrum_solidity_version_test.sol +++ /dev/null @@ -1,4 +0,0 @@ -// SPDX-License-Identifier: SEE LICENSE IN LICENSE -pragma solidity 0.8.21; - -contract Test {} From b4ba7f337c697c71293e9a4ddeb201ed8f59786e Mon Sep 17 00:00:00 2001 From: nikolay19 Date: Mon, 26 Feb 2024 15:53:15 +0300 Subject: [PATCH 17/48] bench updates --- slitherin-benchmark | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/slitherin-benchmark b/slitherin-benchmark index 96f43d6..0690155 160000 --- a/slitherin-benchmark +++ b/slitherin-benchmark @@ -1 +1 @@ -Subproject commit 96f43d6b35076a2b2912d9dfbc9695de0c8dfede +Subproject commit 069015590cdd75f1348a1f78d1c9d1ba84021dff From 21a5ea6e58362ec25fd6b22a849d8e9309fcd189 Mon Sep 17 00:00:00 2001 From: Yhtyyar Sahatov Date: Mon, 4 Mar 2024 23:04:35 +0300 Subject: [PATCH 18/48] added detector --- slitherin/__init__.py | 2 + .../arbitrum/arbitrum_chainlink_price_feed.py | 67 +++++++++++++++++++ 2 files changed, 69 insertions(+) create mode 100644 slitherin/detectors/arbitrum/arbitrum_chainlink_price_feed.py diff --git a/slitherin/__init__.py b/slitherin/__init__.py index 35c1218..3fdcdad 100644 --- a/slitherin/__init__.py +++ b/slitherin/__init__.py @@ -30,12 +30,14 @@ from slitherin.detectors.arbitrum.block_number_timestamp import ( ArbitrumBlockNumberTimestamp, ) +from slitherin.detectors.arbitrum.arbitrum_chainlink_price_feed import ArbitrumChainlinkPriceFeed from slitherin.detectors.potential_arith_overflow import PotentialArithmOverflow artbitrum_detectors = [ ArbitrumPrevrandaoDifficulty, ArbitrumSolidityVersion, ArbitrumBlockNumberTimestamp, + ArbitrumChainlinkPriceFeed ] plugin_detectors = artbitrum_detectors + [ diff --git a/slitherin/detectors/arbitrum/arbitrum_chainlink_price_feed.py b/slitherin/detectors/arbitrum/arbitrum_chainlink_price_feed.py new file mode 100644 index 0000000..4683a10 --- /dev/null +++ b/slitherin/detectors/arbitrum/arbitrum_chainlink_price_feed.py @@ -0,0 +1,67 @@ +import os +import re +from typing import List +from slither.utils.output import Output +from slither.detectors.abstract_detector import AbstractDetector, DetectorClassification +from slither.core.declarations import Function +from slither.analyses.data_dependency.data_dependency import is_dependent + + +from ...consts import ARBITRUM_KEY + + +class ArbitrumChainlinkPriceFeed(AbstractDetector): + """ + Checks if sequencer uptime is verified when chainlink price feed is used + """ + + ARGUMENT = "pess-arb-chainlink-price-feed" # slither will launch the detector with slither.py --detect mydetector + + HELP = "Sequencer uptime is not checked" + IMPACT = DetectorClassification.MEDIUM + CONFIDENCE = DetectorClassification.MEDIUM + + WIKI = "https://github.com/pessimistic-io/slitherin/blob/master/docs/arb_chainlink_price_feed.md" + WIKI_TITLE = "Arbitrum chainlink sequencer uptime" + WIKI_DESCRIPTION = "Arbitrum chainlink sequencer uptime check" + WIKI_EXPLOIT_SCENARIO = "N/A" + WIKI_RECOMMENDATION = "Check sequencer uptime (see docs)" + + def _contains_latest_round_call(self, f: Function) -> bool: + ret = set() + for node in f.nodes: + for _, v in node.high_level_calls: + if isinstance(v, Function) and v.name == "latestRoundData": + return True + return False + + def _contains_sequencer_check(self, f: Function) -> bool: + # Checks if the keyword sequencer used in function. + # This check might contain FPs. However, since most of devs will copy-paste or get inspiration + # from the chainlink docs, as for now, I think it should work well. + + for node in f.nodes: + if re.search(r"sequencer", str(node), re.IGNORECASE): + return True + + return False + + def _detect(self) -> List[Output]: + """Main function""" + results = [] + + if os.getenv(ARBITRUM_KEY) is None: + return results + + for contract in self.compilation_unit.contracts_derived: + for f in contract.functions_and_modifiers: + if self._contains_latest_round_call( + f + ) and not self._contains_sequencer_check(f): + info = [ + "Sequencer uptime status is not checked when using price feed in:\n\t", + f, + "\nCheck https://docs.chain.link/data-feeds/l2-sequencer-feeds for more details\n", + ] + results.append(self.generate_result(info)) + return results From 5b3a52c54c850273e574a466c295b30ee92e8804 Mon Sep 17 00:00:00 2001 From: Yhtyyar Sahatov Date: Mon, 4 Mar 2024 23:05:03 +0300 Subject: [PATCH 19/48] tets for arb-chainlink detector --- tests/arbitrum_chainlink_pricefeed_test.sol | 88 +++++++++++++++++++ tests/interfaces/IAggregatorV2V3Interface.sol | 37 ++++++++ 2 files changed, 125 insertions(+) create mode 100644 tests/arbitrum_chainlink_pricefeed_test.sol create mode 100644 tests/interfaces/IAggregatorV2V3Interface.sol diff --git a/tests/arbitrum_chainlink_pricefeed_test.sol b/tests/arbitrum_chainlink_pricefeed_test.sol new file mode 100644 index 0000000..7f9d552 --- /dev/null +++ b/tests/arbitrum_chainlink_pricefeed_test.sol @@ -0,0 +1,88 @@ +// SPDX-License-Identifier: MIT +pragma solidity ^0.8.7; + +import "./interfaces/IAggregatorV2V3Interface.sol"; + +/** + * THIS IS AN EXAMPLE CONTRACT THAT USES HARDCODED VALUES FOR CLARITY. + * THIS IS AN EXAMPLE CONTRACT THAT USES UN-AUDITED CODE. + * DO NOT USE THIS CODE IN PRODUCTION. + */ + +contract DataConsumerWithSequencerCheck { + AggregatorV2V3Interface internal dataFeed; + AggregatorV2V3Interface internal sequencerUptimeFeed; + + uint256 private constant GRACE_PERIOD_TIME = 3600; + + error SequencerDown(); + error GracePeriodNotOver(); + + /** + * Network: Optimism Goerli testnet + * Data Feed: BTC/USD + * Data Feed address: 0xC16679B963CeB52089aD2d95312A5b85E318e9d2 + * Uptime Feed address: 0x4C4814aa04433e0FB31310379a4D6946D5e1D353 + * For a list of available Sequencer Uptime Feed proxy addresses, see: + * https://docs.chain.link/docs/data-feeds/l2-sequencer-feeds + */ + constructor() { + dataFeed = AggregatorV2V3Interface( + 0xC16679B963CeB52089aD2d95312A5b85E318e9d2 + ); + sequencerUptimeFeed = AggregatorV2V3Interface( + 0x4C4814aa04433e0FB31310379a4D6946D5e1D353 + ); + } + + // Check the sequencer status and return the latest data + function getFeedLatestOk() public view returns (int) { + // prettier-ignore + ( + /*uint80 roundID*/, + int256 answer, + uint256 startedAt, + /*uint256 updatedAt*/, + /*uint80 answeredInRound*/ + ) = sequencerUptimeFeed.latestRoundData(); + + // Answer == 0: Sequencer is up + // Answer == 1: Sequencer is down + bool isSequencerUp = answer == 0; + if (!isSequencerUp) { + revert SequencerDown(); + } + + // Make sure the grace period has passed after the + // sequencer is back up. + uint256 timeSinceUp = block.timestamp - startedAt; + if (timeSinceUp <= GRACE_PERIOD_TIME) { + revert GracePeriodNotOver(); + } + + // prettier-ignore + ( + /*uint80 roundID*/, + int data, + /*uint startedAt*/, + /*uint timeStamp*/, + /*uint80 answeredInRound*/ + ) = dataFeed.latestRoundData(); + + return data; + } + + function getFeedLatestVuln() public view returns (int) { + + // prettier-ignore + ( + /*uint80 roundID*/, + int data, + /*uint startedAt*/, + /*uint timeStamp*/, + /*uint80 answeredInRound*/ + ) = dataFeed.latestRoundData(); + + return data; + } +} \ No newline at end of file diff --git a/tests/interfaces/IAggregatorV2V3Interface.sol b/tests/interfaces/IAggregatorV2V3Interface.sol new file mode 100644 index 0000000..d142f86 --- /dev/null +++ b/tests/interfaces/IAggregatorV2V3Interface.sol @@ -0,0 +1,37 @@ +// SPDX-License-Identifier: MIT +pragma solidity ^0.8.0; +interface AggregatorInterface { + function latestAnswer() external view returns (int256); + + function latestTimestamp() external view returns (uint256); + + function latestRound() external view returns (uint256); + + function getAnswer(uint256 roundId) external view returns (int256); + + function getTimestamp(uint256 roundId) external view returns (uint256); + + event AnswerUpdated(int256 indexed current, uint256 indexed roundId, uint256 updatedAt); + + event NewRound(uint256 indexed roundId, address indexed startedBy, uint256 startedAt); +} + + +interface AggregatorV3Interface { + function decimals() external view returns (uint8); + + function description() external view returns (string memory); + + function version() external view returns (uint256); + + function getRoundData( + uint80 _roundId + ) external view returns (uint80 roundId, int256 answer, uint256 startedAt, uint256 updatedAt, uint80 answeredInRound); + + function latestRoundData() + external + view + returns (uint80 roundId, int256 answer, uint256 startedAt, uint256 updatedAt, uint80 answeredInRound); +} + +interface AggregatorV2V3Interface is AggregatorInterface, AggregatorV3Interface {} \ No newline at end of file From f5769a3f4da05b67a47cd6e4144d4e320fe5bcf1 Mon Sep 17 00:00:00 2001 From: Yhtyyar Sahatov Date: Mon, 4 Mar 2024 23:05:27 +0300 Subject: [PATCH 20/48] docs for arb-chainlink detector --- docs/arb_chainlink_price_feed.md | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) create mode 100644 docs/arb_chainlink_price_feed.md diff --git a/docs/arb_chainlink_price_feed.md b/docs/arb_chainlink_price_feed.md new file mode 100644 index 0000000..f5ad0fb --- /dev/null +++ b/docs/arb_chainlink_price_feed.md @@ -0,0 +1,18 @@ +# Arbitrum chainlink sequencer uptime +## Configuration + +- Check: `pess-arb-chainlink-price-feed` +- Severity: `Medium` +- Confidence: `Medium` + +## Description + +Sequencer uptime status should be checked. For details: [chainlink docs](https://docs.chain.link/data-feeds/l2-sequencer-feeds) + +## Vulnerable Scenario + +[test scenarios](../tests/arbitrum_chainlink_pricefeed_test.sol) + +## Recommendation + +Verify, sequencer uptmie From b4eafaa7e14d263418b6daeb6466abe6338d5f5b Mon Sep 17 00:00:00 2001 From: Yhtyyar Sahatov Date: Mon, 4 Mar 2024 23:11:44 +0300 Subject: [PATCH 21/48] removing uniswap v3 folder, as it was accidentally added --- slitherin/detectors/uniswap-v3/callback.py | 99 ---------------------- 1 file changed, 99 deletions(-) delete mode 100644 slitherin/detectors/uniswap-v3/callback.py diff --git a/slitherin/detectors/uniswap-v3/callback.py b/slitherin/detectors/uniswap-v3/callback.py deleted file mode 100644 index 4681a40..0000000 --- a/slitherin/detectors/uniswap-v3/callback.py +++ /dev/null @@ -1,99 +0,0 @@ -import re -from typing import List, Tuple - -from slither.detectors.abstract_detector import AbstractDetector, DetectorClassification -from slither.slithir.operations import SolidityCall, Condition -from slither.core.declarations import ( - FunctionContract, - SolidityVariableComposed, - Function, -) -from slither.core.cfg.node import Node -from slither.slithir.operations import LowLevelCall -from slither.analyses.data_dependency.data_dependency import is_dependent - - -class AAVEFlashloanCallbackDetector(AbstractDetector): - """ - Detects if the flashloan callback `executeOperation` has `initiator` and `msg.sender` validation - """ - - ARGUMENT = "pess-aave-flashloan-callback" # slither will launch the detector with slither.py --detect mydetector - HELP = "see `executeOperation`callback docs" - IMPACT = DetectorClassification.HIGH - CONFIDENCE = DetectorClassification.HIGH - - WIKI = "https://github.com/pessimistic-io/slitherin/blob/master/docs/aave/flashloan_callback.md" - WIKI_TITLE = "AAVE Flashloan callback" - WIKI_DESCRIPTION = "Check docs" - WIKI_EXPLOIT_SCENARIO = ( - "Attacker can directly call or initiate flashloan to this address" - ) - WIKI_RECOMMENDATION = "Validate the msg.sender and `initiator` argument" - - def _analyze_function( - self, - function: FunctionContract, - args_to_verify, - check_recursive=False, - visited=[], - ) -> Tuple[set, set]: - checked = set() - unchecked = set(args_to_verify) - - for node in function.nodes: - if node.is_conditional(include_loop=False): # contains if/assert/require - for a in unchecked: - if any([is_dependent(arg, a, node) for arg in node.variables_read]): - checked.add(a) - - unchecked.difference_update(checked) # remove all checked - - visited.append(function) - if check_recursive: - for node in function.nodes: - for internal_call in node.internal_calls: - # Filter to Function, as internal_call can be a solidity call - if isinstance(internal_call, Function): - _checked, _unchecked = self._analyze_function( - internal_call, unchecked, False, visited - ) # We are going only 1 step into in recursion - checked |= _checked - unchecked = _unchecked - - return checked, unchecked - - def _detect(self): - results = [] - for contract in self.compilation_unit.contracts_derived: - for f in contract.functions: - if ( - f.signature_str - == "executeOperation(address[],uint256[],uint256[],address,bytes) returns(bool)" - ): - _, unchecked = self._analyze_function( - f, - [ - SolidityVariableComposed("msg.sender"), - f.parameters[3], # initiator parameter - ], - check_recursive=True, - ) - if unchecked: - info = [ - "Unchecked function parameters in AAVE callback: ", - f, - "\n", - ] - for var in unchecked: - if var == SolidityVariableComposed("msg.sender"): - info += ["\t", "'msg.sender' is not checked", "\n"] - else: - info += ["\t'", var.name, "' is not checked\n"] - - tres = self.generate_result(info) - # for var in unchecked: - # tres.add(var) - # tres.add(f) - results.append(tres) - return results From fea92e44f423e31dbc973e3cce4707658ebffe18 Mon Sep 17 00:00:00 2001 From: Yhtyyar Sahatov Date: Tue, 5 Mar 2024 10:55:20 +0300 Subject: [PATCH 22/48] added curve readonly detector --- slitherin/__init__.py | 4 +- .../curve/curve_readonly_reentrancy.py | 113 ++++++++++++++++++ tests/curve_readonly_reentrancy_test.sol | 23 ++++ 3 files changed, 139 insertions(+), 1 deletion(-) create mode 100644 slitherin/detectors/curve/curve_readonly_reentrancy.py create mode 100644 tests/curve_readonly_reentrancy_test.sol diff --git a/slitherin/__init__.py b/slitherin/__init__.py index cd9e78b..31f4943 100644 --- a/slitherin/__init__.py +++ b/slitherin/__init__.py @@ -30,6 +30,7 @@ ArbitrumBlockNumberTimestamp, ) from slitherin.detectors.potential_arith_overflow import PotentialArithmOverflow +from slitherin.detectors.curve.curve_readonly_reentrancy import CurveReadonlyReentrancy artbitrum_detectors = [ ArbitrumPrevrandaoDifficulty, @@ -60,7 +61,8 @@ Ecrecover, PublicVsExternal, AAVEFlashloanCallbackDetector, - PotentialArithmOverflow + PotentialArithmOverflow, + CurveReadonlyReentrancy, ] plugin_printers = [] diff --git a/slitherin/detectors/curve/curve_readonly_reentrancy.py b/slitherin/detectors/curve/curve_readonly_reentrancy.py new file mode 100644 index 0000000..a571aee --- /dev/null +++ b/slitherin/detectors/curve/curve_readonly_reentrancy.py @@ -0,0 +1,113 @@ +from typing import List +from slither.utils.output import Output +from slither.detectors.abstract_detector import AbstractDetector, DetectorClassification +from slither.core.declarations import Function, Contract +from slither.core.cfg.node import Node +import re + + +class CurveReadonlyReentrancy(AbstractDetector): + """ + Sees if a contract has a beforeTokenTransfer function. + """ + + ARGUMENT = "pess-curve-readonly-reentrancy" # slither will launch the detector with slither.py --detect mydetector + HELP = "Curve readonly-reentrancy" + IMPACT = DetectorClassification.HIGH + CONFIDENCE = DetectorClassification.MEDIUM + + WIKI = "https://github.com/pessimistic-io/slitherin/blob/master/docs/balancer/readonly_reentrancy.md" + WIKI_TITLE = "Curve Readonly Reentrancy" + WIKI_DESCRIPTION = "Check docs" + WIKI_EXPLOIT_SCENARIO = "-" + WIKI_RECOMMENDATION = "Check docs" + + VULNERABLE_FUNCTION_CALLS = ["get_virtual_price", "lp_price"] + visited = [] + contains_reentrancy_check = {} + + def is_curve_integration(self, c: Contract) -> bool: + """ + Iterates over all external function calls, and checks the interface/contract name + for a specific keywords to decide if the contract integrates with balancer + + @note as for now, we are skipping any name/interface checks + """ + + return True + + def _has_reentrancy_check(self, node: Node) -> bool: + if node in self.visited: + return self.contains_reentrancy_check[node] + + self.visited.append(node) + self.contains_reentrancy_check[node] = False + + for _, n in node.high_level_calls: + if isinstance(n, Function): + if not n.name: + continue + if n.name == "withdraw_admin_fee": + self.contains_reentrancy_check[node] = True + return True + + # TODO: currently using regex to find address().call(*.withdraw_admin_fees) + # It would be better, if it is rewritten in a ast analysis form + + if re.search(r"withdraw_admin_fee", str(node)): + return True + + has_check = False + for internal_call in node.internal_calls: + if isinstance(internal_call, Function): + for f_node in internal_call.nodes: + has_check |= self._has_reentrancy_check(f_node) + + self.contains_reentrancy_check[node] = has_check + return has_check + + def _check_function(self, function: Function) -> list: + has_dangerous_call = False + dangerous_call = None + for n in function.nodes: + for c, fc in n.high_level_calls: + if isinstance(fc, Function): + if fc.name in self.VULNERABLE_FUNCTION_CALLS: + dangerous_call = n # Saving only first dangerous call + has_dangerous_call = True + break + + if has_dangerous_call and not any( + [self._has_reentrancy_check(node) for node in function.nodes] + ): + return [dangerous_call] + return [] + + def _detect(self) -> List[Output]: + """Main function""" + result = [] + for contract in self.compilation_unit.contracts_derived: + if not self.is_curve_integration(contract): + continue + res = [] + for f in contract.functions_and_modifiers_declared: + function_result = self._check_function(f) + if function_result: + res.extend(function_result) + if res: + info = [ + "Curve readonly-reentrancy vulnerability detected in ", + contract, + ":\n", + ] + for r in res: + info += [ + "\tThe answer of ", + r, + " call could be manipulated through readonly-reentrancy\n", + ] + res = self.generate_result(info) + res.add(r) + result.append(res) + + return result diff --git a/tests/curve_readonly_reentrancy_test.sol b/tests/curve_readonly_reentrancy_test.sol new file mode 100644 index 0000000..62718af --- /dev/null +++ b/tests/curve_readonly_reentrancy_test.sol @@ -0,0 +1,23 @@ +// SPDX-License-Identifier: SEE LICENSE IN LICENSE +pragma solidity ^0.8.0; + +interface ICurveLP { + function get_virtual_price() external returns (uint256); + function withdraw_admin_fee() external returns (uint256); +} + +contract CurveLPTest { + ICurveLP lp = ICurveLP(address(0x0)); + + function vuln () external { + uint p = lp.get_virtual_price(); + } + function _ensureNonReentrant() private { + (bool success,) = address(lp).call(abi.encodeWithSelector(ICurveLP.withdraw_admin_fee.selector)); + require(!success, "reentrant call"); + } + function ok() external { + _ensureNonReentrant(); + uint p = lp.get_virtual_price(); + } +} \ No newline at end of file From 5f5555a03a5de6c457b28444ce27dd337d80ab5b Mon Sep 17 00:00:00 2001 From: Yhtyyar Sahatov Date: Wed, 6 Mar 2024 10:36:52 +0300 Subject: [PATCH 23/48] added docs for curve detector --- docs/curve_readonly_reentrancy.md | 24 +++++++++++++++++++ .../curve/curve_readonly_reentrancy.py | 2 +- 2 files changed, 25 insertions(+), 1 deletion(-) create mode 100644 docs/curve_readonly_reentrancy.md diff --git a/docs/curve_readonly_reentrancy.md b/docs/curve_readonly_reentrancy.md new file mode 100644 index 0000000..51a3df4 --- /dev/null +++ b/docs/curve_readonly_reentrancy.md @@ -0,0 +1,24 @@ +# Curve Readonly Reentrancy + +## Configuration + +- Check: `pess-curve-readonly-reentrancy` +- Severity: `High` +- Confidence: `Medium` + +## Description + +Highlights the use of Curve getter functions `get_virtual_price` and `lp_price` (which are not checked for readonly reentrancy `withdraw_admin_fee`), which return values that theoretically could be manipulated during the execution. Details: [Curve LP Oracle Manipulation](https://chainsecurity.com/curve-lp-oracle-manipulation-post-mortem/) + +## Vulnerable Scenario + +[test scenarios](../../tests/curve_readonly_reentrancy_test.sol) + +## Related Attacks + +- [Jarvis Exploit](https://www.google.com/url?q=https://blog.solidityscan.com/jarvis-polygon-pool-hack-analysis-read-only-re-entrancy-af0607e4585a&sa=D&source=editors&ust=1709713964156907&usg=AOvVaw1Oess2f9Z_UCD6vLM2hN26) +- [Market.xyz Exploit](https://quillaudits.medium.com/decoding-220k-read-only-reentrancy-exploit-quillaudits-30871d728ad5) + +## Recomendations + +- Verify by calling `withdraw_admin_fee` and checking for fail of call \ No newline at end of file diff --git a/slitherin/detectors/curve/curve_readonly_reentrancy.py b/slitherin/detectors/curve/curve_readonly_reentrancy.py index a571aee..c3d641a 100644 --- a/slitherin/detectors/curve/curve_readonly_reentrancy.py +++ b/slitherin/detectors/curve/curve_readonly_reentrancy.py @@ -16,7 +16,7 @@ class CurveReadonlyReentrancy(AbstractDetector): IMPACT = DetectorClassification.HIGH CONFIDENCE = DetectorClassification.MEDIUM - WIKI = "https://github.com/pessimistic-io/slitherin/blob/master/docs/balancer/readonly_reentrancy.md" + WIKI = "https://github.com/pessimistic-io/slitherin/blob/master/docs/curve_readonly_reentrancy.md" WIKI_TITLE = "Curve Readonly Reentrancy" WIKI_DESCRIPTION = "Check docs" WIKI_EXPLOIT_SCENARIO = "-" From 709548a9632f71438b7bd927ce70ee6f5ee2f7d3 Mon Sep 17 00:00:00 2001 From: nikolay19 Date: Thu, 7 Mar 2024 15:58:45 +0300 Subject: [PATCH 24/48] PR&slither version to sheet --- .github/workflows/benchmark.yml | 13 ++++++++++--- slitherin-benchmark | 2 +- 2 files changed, 11 insertions(+), 4 deletions(-) diff --git a/.github/workflows/benchmark.yml b/.github/workflows/benchmark.yml index 48cebf0..013bd3a 100644 --- a/.github/workflows/benchmark.yml +++ b/.github/workflows/benchmark.yml @@ -45,20 +45,26 @@ jobs: - name: Run Benchmark run: | cd slitherin-benchmark/ - python runner.py -i contracts/mainnet -o mainnet.csv --limit 8000 --skip-duplicates --skip-libs + python runner.py -i contracts/mainnet -o mainnet.csv -eo mainnet_extra.csv --limit 8000 --skip-duplicates --skip-libs - name: Upload sheet run: | cd slitherin-benchmark/ echo $GOOGLE_JWT > service_account.json - python save_sheet.py -i mainnet.csv -sa service_account.json -si $GOOGLE_SHEET_ID -ln mainnet + python save_sheet.py -i mainnet.csv -sa service_account.json -si $GOOGLE_SHEET_ID -ln mainnet -pr $PR_NUMBER env: GOOGLE_JWT : ${{secrets.SERVICE_ACCOUNT}} GOOGLE_SHEET_ID : ${{ secrets.GOOGLE_SHEET_ID }} + PR_NUMBER: ${{ github.event.number }} - name: 'Upload Artifact' uses: actions/upload-artifact@v3 with: name: mainnet path: slitherin-benchmark/mainnet.csv + - name: 'Upload Artifact Extra' + uses: actions/upload-artifact@v3 + with: + name: mainnet + path: slitherin-benchmark/mainnet_extra.csv RunBenchmarkOZ: runs-on: ubuntu-latest steps: @@ -102,10 +108,11 @@ jobs: run: | cd slitherin-benchmark/ echo $GOOGLE_JWT > service_account.json - python save_sheet.py -i oz.csv -sa service_account.json -si $GOOGLE_SHEET_ID -ln OZ + python save_sheet.py -i oz.csv -sa service_account.json -si $GOOGLE_SHEET_ID -ln OZ -pr $PR_NUMBER env: GOOGLE_JWT : ${{secrets.SERVICE_ACCOUNT}} GOOGLE_SHEET_ID : ${{ secrets.GOOGLE_SHEET_ID }} + PR_NUMBER: ${{ github.event.number }} - name: 'Upload Artifact' uses: actions/upload-artifact@v3 with: diff --git a/slitherin-benchmark b/slitherin-benchmark index 0690155..e8a7b64 160000 --- a/slitherin-benchmark +++ b/slitherin-benchmark @@ -1 +1 @@ -Subproject commit 069015590cdd75f1348a1f78d1c9d1ba84021dff +Subproject commit e8a7b64b52af91eb257399cdaf16c68c5695d9d7 From 6bd5782fcbcce4d4596d1dd268f38edb42c0efdb Mon Sep 17 00:00:00 2001 From: olegggatttor Date: Mon, 12 Feb 2024 09:21:01 +0200 Subject: [PATCH 25/48] add erc1155 support --- docs/nft_approve_warning.md | 4 +-- slitherin/detectors/nft_approve_warning.py | 3 ++- tests/nft_approve_warning_test.sol | 29 ++++++++++++++++++++++ 3 files changed, 33 insertions(+), 3 deletions(-) diff --git a/docs/nft_approve_warning.md b/docs/nft_approve_warning.md index bb52d84..ffba3ee 100644 --- a/docs/nft_approve_warning.md +++ b/docs/nft_approve_warning.md @@ -6,7 +6,7 @@ * Confidence: `Low` ## Description -The detector sees if a contract contains `erc721.[safe]TransferFrom(from, ...)` where `from` parameter is not related to `msg.sender`. +The detector sees if a contract contains `erc721.[safe]TransferFrom(from, ...)` or `erc1155.safe[Batch]TransferFrom(from, ...)` where `from` parameter is not related to `msg.sender`. An attacker can steal any approved NFTs because `transferFrom` function does NOT check that the call is made by its owner. ## Vulnerable Scenario @@ -17,4 +17,4 @@ An attacker can steal any approved NFTs because `transferFrom` function does NOT [Unauthorized transfer_from Vulnerability](https://ventral.digital/posts/2022/8/18/sznsdaos-bountyboard-unauthorized-transferfrom-vulnerability) ## Recommendation -Make sure that in `erc721.[safe]TransferFrom(from, ...)` functions `from` parameter is related to `msg.sender`. +Make sure that in `erc721.[safe]TransferFrom(from, ...)` and `erc1155.safe[Batch]TransferFrom(from, ...)` functions `from` parameter is related to `msg.sender`. diff --git a/slitherin/detectors/nft_approve_warning.py b/slitherin/detectors/nft_approve_warning.py index 8f17299..b74f968 100644 --- a/slitherin/detectors/nft_approve_warning.py +++ b/slitherin/detectors/nft_approve_warning.py @@ -21,7 +21,8 @@ class NftApproveWarning(AbstractDetector): WIKI_EXPLOIT_SCENARIO = '-' WIKI_RECOMMENDATION = 'from parameter must be related to msg.sender' - _signatures=["transferFrom(address,address,uint256)", "safeTransferFrom(address,address,uint256,bytes)", "safeTransferFrom(address,address,uint256)"] + _signatures=["transferFrom(address,address,uint256)", "safeTransferFrom(address,address,uint256,bytes)", "safeTransferFrom(address,address,uint256)", + "safeTransferFrom(address,address,uint256,uint256,bytes)", "safeBatchTransferFrom(address,address,uint256[],uint256[],bytes)"] # ERC1155 def _detect_arbitrary_from(self, f: Function): all_high_level_calls = [ diff --git a/tests/nft_approve_warning_test.sol b/tests/nft_approve_warning_test.sol index a53f7b0..6c9df9a 100644 --- a/tests/nft_approve_warning_test.sol +++ b/tests/nft_approve_warning_test.sol @@ -36,3 +36,32 @@ contract Test } } +interface IERC1155 { + function safeTransferFrom(address from, address to, uint256 id, uint256 value, bytes memory data) external; + function safeBatchTransferFrom( + address from, + address to, + uint256[] memory ids, + uint256[] memory values, + bytes memory data + ) external; +} + +contract TestERC1155 { + function vuln_safeTransferFrom(address target, address from, address to) external { + IERC1155(target).safeTransferFrom(from, to, 1, 1, ""); + } + + function ok_safeTransferFrom(address target, address to) external { + IERC1155(target).safeTransferFrom(msg.sender, to, 1, 1, ""); + } + + function vuln_safeBatchTransferFrom(address target, address from, address to) external { + IERC1155(target).safeBatchTransferFrom(from, to, new uint256[](2), new uint256[](2), ""); + } + + function ok_safeBatchTransferFrom(address target, address to) external { + IERC1155(target).safeBatchTransferFrom(msg.sender, to, new uint256[](2), new uint256[](2), ""); + } +} + From 982b7ffadbe0b9caa5eaaab1e77bc23653a805fc Mon Sep 17 00:00:00 2001 From: olegggatttor Date: Mon, 12 Feb 2024 09:46:48 +0200 Subject: [PATCH 26/48] add SafeERC20 support --- slitherin/detectors/nft_approve_warning.py | 11 ++++++---- tests/nft_approve_warning_test.sol | 24 ++++++++++++++++++++++ 2 files changed, 31 insertions(+), 4 deletions(-) diff --git a/slitherin/detectors/nft_approve_warning.py b/slitherin/detectors/nft_approve_warning.py index b74f968..d7aef4a 100644 --- a/slitherin/detectors/nft_approve_warning.py +++ b/slitherin/detectors/nft_approve_warning.py @@ -2,9 +2,10 @@ from typing import List from slither.core.cfg.node import Node from slither.core.declarations import Function, SolidityVariableComposed +from slither.core.declarations.function_contract import FunctionContract from slither.analyses.data_dependency.data_dependency import is_dependent - +SAFE_ERC20_LIB_SIG = "safeTransferFrom(address,address,address,uint256)" class NftApproveWarning(AbstractDetector): """ Sees if contract contains erc721.[safe]TransferFrom(from, ...) where from parameter is not related to msg.sender @@ -16,13 +17,14 @@ class NftApproveWarning(AbstractDetector): CONFIDENCE = DetectorClassification.LOW WIKI = 'https://ventral.digital/posts/2022/8/18/sznsdaos-bountyboard-unauthorized-transferfrom-vulnerability' - WIKI_TITLE = 'NFT Approve Warning' + WIKI_TITLE = 'Token Approve Warning' WIKI_DESCRIPTION = "In [safe]TransferFrom() from parameter must be related to msg.sender" WIKI_EXPLOIT_SCENARIO = '-' WIKI_RECOMMENDATION = 'from parameter must be related to msg.sender' _signatures=["transferFrom(address,address,uint256)", "safeTransferFrom(address,address,uint256,bytes)", "safeTransferFrom(address,address,uint256)", - "safeTransferFrom(address,address,uint256,uint256,bytes)", "safeBatchTransferFrom(address,address,uint256[],uint256[],bytes)"] # ERC1155 + SAFE_ERC20_LIB_SIG, # SafeERC20.safeTransferFrom + "safeTransferFrom(address,address,uint256,uint256,bytes)", "safeBatchTransferFrom(address,address,uint256[],uint256[],bytes)"] # ERC1155 def _detect_arbitrary_from(self, f: Function): all_high_level_calls = [ @@ -49,7 +51,8 @@ def _arbitrary_from(self, nodes: List[Node]): and hasattr(ir.function, 'solidity_signature') and ir.function.solidity_signature in self._signatures ): - is_from_sender = is_dependent(ir.arguments[0], SolidityVariableComposed("msg.sender"), node.function.contract) + is_from_sender = ir.function.solidity_signature != SAFE_ERC20_LIB_SIG and is_dependent(ir.arguments[0], SolidityVariableComposed("msg.sender"), node.function.contract) or \ + ir.function.solidity_signature == SAFE_ERC20_LIB_SIG and is_dependent(ir.arguments[1], SolidityVariableComposed("msg.sender"), node.function.contract) # is_from_self = is_dependent(ir.arguments[0], SolidityVariable("this"), node.function.contract) if (not is_from_sender): # and not is_from_self irList.append(ir.node) diff --git a/tests/nft_approve_warning_test.sol b/tests/nft_approve_warning_test.sol index 6c9df9a..735e0b9 100644 --- a/tests/nft_approve_warning_test.sol +++ b/tests/nft_approve_warning_test.sol @@ -65,3 +65,27 @@ contract TestERC1155 { } } + +library SafeERC20 { + function safeTransferFrom(IERC20 token, address from, address to, uint256 value) internal { + // _callOptionalReturn(token, abi.encodeCall(token.transferFrom, (from, to, value))); + } +} + +contract TestSafeERC20LibCall { + function vuln1_safeErc20TransferFrom(IERC20 token, address from, address to, uint256 value) external { + SafeERC20.safeTransferFrom(token, from, to, value); + } + + function ok1_safeErc20TransferFrom(IERC20 token, address to, uint256 value) external { + SafeERC20.safeTransferFrom(token, msg.sender, to, value); + } + + function vuln2_safeErc20TransferFrom(address from, address to, uint256 value) external { + SafeERC20.safeTransferFrom(IERC20(msg.sender), from, to, value); + } + + function ok2_safeErc20TransferFrom(address to, uint256 value) external { + SafeERC20.safeTransferFrom(IERC20(msg.sender), msg.sender, to, value); + } +} From c1b067d4443c774d4afad4921a6abb0b3a9cd324 Mon Sep 17 00:00:00 2001 From: Nikita Kirillov Date: Fri, 22 Mar 2024 14:16:49 +0500 Subject: [PATCH 27/48] corrected_readme --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index c3772bd..138795a 100644 --- a/README.md +++ b/README.md @@ -20,7 +20,7 @@ Please let us know if you have discovered an [issue/bug/vulnerability](https://t | Section | Link | | ---------------------------- | ----------------------------------------------------------------------------------------------- | | Docs | [Docs for each detector](https://github.com/pessimistic-io/slitherin/tree/master/docs) | -| Slither_pess | [Detectors code](https://github.com/pessimistic-io/slitherin/tree/master/slither_pess) | +| Slitherin | [Detectors code](https://github.com/pessimistic-io/slitherin/tree/master/slitherin) | | Tests | [Test contracts for detectors](https://github.com/pessimistic-io/slitherin/tree/master/tests) | | Utils | [Auxiliary files](https://github.com/pessimistic-io/slitherin/tree/master/utils) | | Issues | [Suggest an idea](https://github.com/pessimistic-io/slitherin/issues) | From a01fdbcdf47bf9cfae005a81fca23b39cacb2043 Mon Sep 17 00:00:00 2001 From: Nikita Kirillov Date: Fri, 22 Mar 2024 14:21:19 +0500 Subject: [PATCH 28/48] readme_from_master --- README.md | 50 ++++++++++++++++++++------------------------------ 1 file changed, 20 insertions(+), 30 deletions(-) diff --git a/README.md b/README.md index 138795a..b15d605 100644 --- a/README.md +++ b/README.md @@ -6,7 +6,7 @@ **Welcome!** We are the [pessimistic.io](https://pessimistic.io/) team, and in recent months we have been actively developing our [own **Slither detectors**](https://github.com/pessimistic-io/slitherin/tree/develop/slitherin/detectors) to help with code review and audit process. This repository contains everything you may require to work with them! -We increased the sensitivity of our detectors since they are _quite straightforward_ and not written in the "original style." As a result, they produce FPs ([False Positives](https://en.wikipedia.org/wiki/False_positives_and_false_negatives)) more frequently than original ones. So that, our detectors are a kind of automation of the checks implemented in the checklist, their main purpose is to look for issues and assist the code auditor. +We increased the sensitivity of our detectors since they are *quite straightforward* and not written in the "original style." As a result, they produce FPs ([False Positives](https://en.wikipedia.org/wiki/False_positives_and_false_negatives)) more frequently than original ones. So that, our detectors are a kind of automation of the checks implemented in the checklist, their main purpose is to look for issues and assist the code auditor. Please let us know if you have discovered an [issue/bug/vulnerability](https://telegra.ph/BountyCTF-Platforms-Web3-04-19) via our custom Slither detectors. You may contact us via opening a [PR/Issue](https://github.com/pessimistic-io/slitherin/issues) or [directly](mailto:gm@pessimistic.io), whichever is more convenient for you. If you have any further questions or suggestions, please [join our Discord Server](https://discord.gg/vPxkR8B9p7) or [Telegram chat](https://t.me/+G96ejJ7Pmgk1NDZi)! We hope to see you there, and we intend to support the community and its initiatives! @@ -17,27 +17,23 @@ Please let us know if you have discovered an [issue/bug/vulnerability](https://t #### **Table of Contents:** -| Section | Link | -| ---------------------------- | ----------------------------------------------------------------------------------------------- | -| Docs | [Docs for each detector](https://github.com/pessimistic-io/slitherin/tree/master/docs) | -| Slitherin | [Detectors code](https://github.com/pessimistic-io/slitherin/tree/master/slitherin) | -| Tests | [Test contracts for detectors](https://github.com/pessimistic-io/slitherin/tree/master/tests) | -| Utils | [Auxiliary files](https://github.com/pessimistic-io/slitherin/tree/master/utils) | -| Issues | [Suggest an idea](https://github.com/pessimistic-io/slitherin/issues) | -| Installation Process | [Step-by-Step guide](https://github.com/pessimistic-io/slitherin#installation-process) | -| Detectors | [Detectors table](https://github.com/pessimistic-io/slitherin#detectors-table) | -| Enhancements & New Detectors | [Project Improvements](https://github.com/pessimistic-io/slitherin#enhancements--new-detectors) | +| Section | Link | +|------------------------------|---------------------------------------------------------------------------------------------------------------| +| Docs | [Docs for each detector](https://github.com/pessimistic-io/slitherin/tree/master/docs) | +| Slitherin | [Detectors code](https://github.com/pessimistic-io/slitherin/tree/master/slitherin/detectors) | +| Tests | [Test contracts for detectors](https://github.com/pessimistic-io/slitherin/tree/master/tests) | +| Utils | [Auxiliary files](https://github.com/pessimistic-io/slitherin/tree/master/slitherin/utils) | +| Issues | [Suggest an idea](https://github.com/pessimistic-io/slitherin/issues) | +| Installation Process | [Step-by-Step guide](https://github.com/pessimistic-io/slitherin#installation-process) | +| Detectors | [Detectors table](https://github.com/pessimistic-io/slitherin#detectors-table) | +| Enhancements & New Detectors | [Project Improvements](https://github.com/pessimistic-io/slitherin#enhancements--new-detectors) | ## Installation Process - ### Using Git - -To install Pessimistic Detectors: - +To install Pessimistic Detectors: 1. Install the [original Slither](https://github.com/crytic/slither#how-to-install); 2. Clone our repository; 3. Run the following command in our repository folder to add new detectors to Slither: - ```bash python3 setup.py develop ``` @@ -46,12 +42,9 @@ python3 setup.py develop ```bash npm install ``` - ### Using Pip - 1. Install the [original Slither](https://github.com/crytic/slither#how-to-install); 2. Install the pip [package](https://pypi.org/project/slitherin/): - ```bash pip install slitherin ``` @@ -109,29 +102,27 @@ Slitherin detectors are included into original Slither after the installation. Y **Please note:** -- \*Valid - issues included in reports and fixed by developers (January 2023 - June 2023). +- *Valid - issues included in reports and fixed by developers (January 2023 - June 2023). - There are two detectors which have several checks inside: [pess-uni-v2](https://github.com/pessimistic-io/slitherin/blob/master/slitherin/detectors/uni_v2.py) and [arbitrary-call](https://github.com/pessimistic-io/slitherin/blob/master/slitherin/detectors/arbitrary_call/arbitrary_call.py). ## Enhancements & New Detectors -Here we indicate our updates, workflows and mark completed tasks and improvements! +Here we indicate our updates, workflows and mark completed tasks and improvements! -> You can add your own _detector/idea/enhancement_ by [opening the Issue at the following link](https://github.com/pessimistic-io/slitherin/issues). +> You can add your own *detector/idea/enhancement* by [opening the Issue at the following link](https://github.com/pessimistic-io/slitherin/issues). -Prior to adding a custom _detector_, ensure that: +Prior to adding a custom *detector*, ensure that: 1. In a documentation file, your detector is comprehensively described; 2. The detector test contract is presented and correctly compiles; 3. The detector code is presented and works properly. -Prior to adding an _idea_, ensure that: - +Prior to adding an *idea*, ensure that: 1. Your concept or idea is well articulated; 2. A vulnerability example (or PoC) is provided; -Prior to adding an _enhancement_, ensure that: - +Prior to adding an *enhancement*, ensure that: 1. Your enhancement does **not** make the base code worse; 2. Your enhancement is commented. @@ -170,11 +161,10 @@ Our team would like to express our deepest gratitude to the [Slither tool](https [![Mail](https://img.shields.io/badge/Mail-gm%40pessimistic.io-orange?style=flat-square&logo=appveyor?logo=data:https://pessimistic.io/favicon.ico)](mailto:gm@pessimistic.io) --- - > Pessimistic delivers trusted security audits since 2017. -> \ +\ > Require expert oversight of your safety? -> \ +\ > Explore our services at [pessimistic.io](https://pessimistic.io/). # From fb08e5247917ba80938e5793bff1bb4035c44511 Mon Sep 17 00:00:00 2001 From: Nikita Kirillov Date: Fri, 22 Mar 2024 14:28:12 +0500 Subject: [PATCH 29/48] replaced_balancer_detector --- .../detectors/readonly_reentrancy/__init__.py | 0 .../read_only_reentrancy.py | 434 ------------------ .../reentrancy/__init__.py | 0 .../reentrancy/reentrancy.py | 314 ------------- slitherin-benchmark | 2 +- .../balancer}/balancer_readonly_reentrancy.py | 0 6 files changed, 1 insertion(+), 749 deletions(-) delete mode 100644 slither_pess/detectors/readonly_reentrancy/__init__.py delete mode 100644 slither_pess/detectors/readonly_reentrancy/read_only_reentrancy.py delete mode 100644 slither_pess/detectors/readonly_reentrancy/reentrancy/__init__.py delete mode 100644 slither_pess/detectors/readonly_reentrancy/reentrancy/reentrancy.py rename {slither_pess/detectors/readonly_reentrancy => slitherin/detectors/balancer}/balancer_readonly_reentrancy.py (100%) diff --git a/slither_pess/detectors/readonly_reentrancy/__init__.py b/slither_pess/detectors/readonly_reentrancy/__init__.py deleted file mode 100644 index e69de29..0000000 diff --git a/slither_pess/detectors/readonly_reentrancy/read_only_reentrancy.py b/slither_pess/detectors/readonly_reentrancy/read_only_reentrancy.py deleted file mode 100644 index 7b17c61..0000000 --- a/slither_pess/detectors/readonly_reentrancy/read_only_reentrancy.py +++ /dev/null @@ -1,434 +0,0 @@ -"""" - Re-entrancy detection - Based on heuristics, it may lead to FP and FN - Iterate over all the nodes of the graph until reaching a fixpoint -""" -from collections import namedtuple, defaultdict -from typing import Dict, List, Set -from slither.core.variables.variable import Variable -from slither.core.declarations import Function -from slither.core.cfg.node import NodeType, Node, Contract -from slither.detectors.abstract_detector import DetectorClassification -from .reentrancy.reentrancy import ( - Reentrancy, - to_hashable, - AbstractState, - union_dict, - _filter_if, - is_subset, -) -from slither.slithir.operations import EventCall - -FindingKey = namedtuple("FindingKey", ["function", "calls"]) -FindingValue = namedtuple("FindingValue", ["variable", "written_at", "node", "nodes"]) - - -def are_same_contract(a: Contract, b: Contract) -> bool: - """ - Checks if A==B or A inherits from B or otherwise - """ - return a == b or (b in a.inheritance) or (b in a.derived_contracts) - - -class ReadOnlyReentrancyState(AbstractState): - def __init__(self): - super().__init__() - self._reads_external: Dict[Variable, Set[Node]] = defaultdict(set) - self._reads_external_contract_list: Dict[Variable, Set[Contract]] = defaultdict( - set - ) - self._written_external: Dict[Variable, Set[Node]] = defaultdict(set) - self._written: Dict[Variable, Set[Node]] = defaultdict(set) - - @property - def reads_external(self) -> Dict[Variable, Set[Node]]: - return self._reads_external - - @property - def reads_external_contract_list(self) -> Dict[Variable, Set[Contract]]: - return self._reads_external_contract_list - - @property - def written_external(self) -> Dict[Variable, Set[Node]]: - return self._written_external - - @property - def written(self) -> Dict[Variable, Set[Node]]: - return self._written - - def add(self, fathers): - super().add(fathers) - self._reads_external = union_dict(self._reads_external, fathers.reads_external) - self._reads_external_contract_list = union_dict( - self._reads_external_contract_list, fathers.reads_external_contract_list - ) - - def does_not_bring_new_info(self, new_info): - return ( - super().does_not_bring_new_info(new_info) - and is_subset(new_info.reads_external, self._reads_external) - and is_subset( - new_info.reads_external_contract_list, - self._reads_external_contract_list, - ) - ) - - def merge_fathers(self, node, skip_father, detector): - for father in node.fathers: - if detector.KEY in father.context: - self._send_eth = union_dict( - self._send_eth, - { - key: values - for key, values in father.context[detector.KEY].send_eth.items() - if key != skip_father - }, - ) - self._calls = union_dict( - self._calls, - { - key: values - for key, values in father.context[detector.KEY].calls.items() - if key != skip_father - }, - ) - self._reads = union_dict( - self._reads, father.context[detector.KEY].reads - ) - self._reads_external = union_dict( - self._reads_external, father.context[detector.KEY].reads - ) - self._written_external = union_dict( - self._written_external, father.context[detector.KEY].reads - ) - - def analyze_node(self, node: Node, detector): - state_vars_read: Dict[Variable, Set[Node]] = defaultdict( - set, {v: {node} for v in node.state_variables_read} - ) - - # All the state variables written - state_vars_written: Dict[Variable, Set[Node]] = defaultdict( - set, {v: {node} for v in node.state_variables_written} - ) - - external_state_vars_read: Dict[Variable, Set[Node]] = defaultdict(set) - external_state_vars_written: Dict[Variable, Set[Node]] = defaultdict(set) - external_state_vars_read_contract_list: Dict[ - Variable, Set[Contract] - ] = defaultdict(set) - - slithir_operations = [] - # Add the state variables written in internal calls - for internal_call in node.internal_calls: - # Filter to Function, as internal_call can be a solidity call - if isinstance(internal_call, Function): - for internal_node in internal_call.all_nodes(): - for read in internal_node.state_variables_read: - state_vars_read[read].add(internal_node) - for write in internal_node.state_variables_written: - state_vars_written[write].add(internal_node) - slithir_operations += internal_call.all_slithir_operations() - - for contract, v in node.high_level_calls: - if isinstance(v, Function): - for internal_node in v.all_nodes(): - for read in internal_node.state_variables_read: - external_state_vars_read[read].add(internal_node) - external_state_vars_read_contract_list[read].add(contract) - - if internal_node.context.get(detector.KEY): - for r in internal_node.context[detector.KEY].reads_external: - external_state_vars_read[r].add(internal_node) - external_state_vars_read_contract_list[r].add(contract) - for write in internal_node.state_variables_written: - external_state_vars_written[write].add(internal_node) - - contains_call = False - - self._written = state_vars_written - self._written_external = external_state_vars_written - for ir in node.irs + slithir_operations: - if detector.can_callback(ir): - self._calls[node] |= {ir.node} - contains_call = True - - if detector.can_send_eth(ir): - self._send_eth[node] |= {ir.node} - - if isinstance(ir, EventCall): - self._events[ir] |= {ir.node, node} - - self._reads = union_dict(self._reads, state_vars_read) - self._reads_external = union_dict( - self._reads_external, external_state_vars_read - ) - self._reads_external_contract_list = union_dict( - self._reads_external_contract_list, external_state_vars_read_contract_list - ) - - return contains_call - - -class ReadOnlyReentrancy(Reentrancy): - ARGUMENT = "pess-readonly-reentrancy" - HELP = "Read-only reentrancy vulnerabilities" - IMPACT = DetectorClassification.HIGH - CONFIDENCE = DetectorClassification.LOW - - WIKI = "https://github.com/pessimistic-io/slitherin/blob/master/docs/readonly_reentrancy.md" - WIKI_TITLE = "Read-only reentrancy vulnerabilities" - WIKI_DESCRIPTION = "Check docs" - STANDARD_JSON = False - KEY = "readonly_reentrancy" - - contracts_read_variable: Dict[Variable, Set[Contract]] = defaultdict(set) - contracts_written_variable_after_reentrancy: Dict[ - Variable, Set[Contract] - ] = defaultdict(set) - - def _explore(self, node, visited, skip_father=None): - if node in visited: - return - - visited = visited + [node] - - fathers_context = ReadOnlyReentrancyState() - fathers_context.merge_fathers(node, skip_father, self) - - # Exclude path that dont bring further information - if node in self.visited_all_paths: - if self.visited_all_paths[node].does_not_bring_new_info(fathers_context): - return - else: - self.visited_all_paths[node] = ReadOnlyReentrancyState() - - self.visited_all_paths[node].add(fathers_context) - - node.context[self.KEY] = fathers_context - - contains_call = fathers_context.analyze_node(node, self) - node.context[self.KEY] = fathers_context - - sons = node.sons - if contains_call and node.type in [NodeType.IF, NodeType.IFLOOP]: - if _filter_if(node): - son = sons[0] - self._explore(son, visited, node) - sons = sons[1:] - else: - son = sons[1] - self._explore(son, visited, node) - sons = [sons[0]] - - for son in sons: - self._explore(son, visited) - - def find_writes_after_reentrancy(self): - written_after_reentrancy: Dict[Variable, Set[Node]] = defaultdict(set) - written_after_reentrancy_external: Dict[Variable, Set[Node]] = defaultdict(set) - for contract in self.contracts: - for f in contract.functions_and_modifiers_declared: - for node in f.nodes: - # dead code - if self.KEY not in node.context: - continue - if node.context[self.KEY].calls: - if not any(n != node for n in node.context[self.KEY].calls): - continue - # TODO: check if written items exist - for v, nodes in node.context[self.KEY].written.items(): - written_after_reentrancy[v].add(node) - self.contracts_written_variable_after_reentrancy[v].add( - contract - ) - for v, nodes in node.context[self.KEY].written_external.items(): - written_after_reentrancy_external[v].add(node) - self.contracts_written_variable_after_reentrancy[v].add( - contract - ) - - return written_after_reentrancy, written_after_reentrancy_external - - # IMPORTANT: - # FOR the external reads, that var should be external written in the same contract - def get_readonly_reentrancies(self): - ( - written_after_reentrancy, - written_after_reentrancy_external, - ) = self.find_writes_after_reentrancy() - result = defaultdict(set) - - warnings = defaultdict(set) - - for contract in self.contracts: - for f in contract.functions_and_modifiers_declared: - for node in f.nodes: - - if self.KEY not in node.context: - continue - vulnerable_variables = set() - warning_variables = set() - for r, nodes in node.context[self.KEY].reads.items(): - - if r in written_after_reentrancy: - finding_value = FindingValue( - r, - tuple( - sorted( - list(written_after_reentrancy[r]), - key=lambda x: x.node_id, - ) - ), - node, - tuple(sorted(nodes, key=lambda x: x.node_id)), - ) - if are_same_contract(r.contract, f.contract): - if f.view and f.visibility in ["public", "external"]: - warning_variables.add(finding_value) - else: - vulnerable_variables.add(finding_value) - - for r, nodes in node.context[self.KEY].reads_external.items(): - if are_same_contract(r.contract, f.contract): - # TODO(yhtiyar): In case f.view we can notify the user that the given - # method could be vulnerable if other contract will use it - continue - if r in written_after_reentrancy_external: - isVulnerable = any( - c in self.contracts_written_variable_after_reentrancy[r] - for c in node.context[ - self.KEY - ].reads_external_contract_list[r] - ) - if isVulnerable: - vulnerable_variables.add( - FindingValue( - r, - tuple( - sorted( - list( - written_after_reentrancy_external[r] - ), - key=lambda x: x.node_id, - ) - ), - node, - tuple(sorted(nodes, key=lambda x: x.node_id)), - ) - ) - - if r in written_after_reentrancy: - vulnerable_variables.add( - FindingValue( - r, - tuple( - sorted( - list(written_after_reentrancy[r]), - key=lambda x: x.node_id, - ) - ), - node, - tuple(sorted(nodes, key=lambda x: x.node_id)), - ) - ) - - if vulnerable_variables: - finding_key = FindingKey( - function=f, calls=to_hashable(node.context[self.KEY].calls) - ) - result[finding_key] |= vulnerable_variables - if warning_variables: - finding_key = FindingKey( - function=f, calls=to_hashable(node.context[self.KEY].calls) - ) - warnings[finding_key] |= warning_variables - return result, warnings - - def _gen_results(self, raw_results, info_text): - results = [] - - result_sorted = sorted( - list(raw_results.items()), key=lambda x: x[0].function.name - ) - - varsRead: List[FindingValue] - for (func, calls), varsRead in result_sorted: - - varsRead = sorted(varsRead, key=lambda x: (x.variable.name, x.node.node_id)) - - info = [f"{info_text} ", func, ":\n"] - - info += [ - "\tState variables read that were written after the external call(s):\n" - ] - for finding_value in varsRead: - info += [ - "\t- ", - finding_value.variable, - " was read at ", - finding_value.node, - "\n", - ] - # info += ["\t- ", finding_value.node, "\n"] - - # for other_node in finding_value.nodes: - # if other_node != finding_value.node: - # info += ["\t\t- ", other_node, "\n"] - - # TODO: currently we are not printing the whole call-stack of variable - # it wasn't working properly, so I am removing it for now to avoid confusion - - info += ["\t\t This variable was written at (after external call):\n"] - for other_node in finding_value.written_at: - # info += ["\t- ", call_info, "\n"] - if other_node != finding_value.node: - info += ["\t\t\t- ", other_node, "\n"] - - # Create our JSON result - res = self.generate_result(info) - - res.add(func) - - # Add all variables written via nodes which write them. - for finding_value in varsRead: - res.add( - finding_value.node, - { - "underlying_type": "variables_written", - "variable_name": finding_value.variable.name, - }, - ) - for other_node in finding_value.nodes: - if other_node != finding_value.node: - res.add( - other_node, - { - "underlying_type": "variables_written", - "variable_name": finding_value.variable.name, - }, - ) - - # Append our result - results.append(res) - - return results - - def _detect(self): # pylint: disable=too-many-branches - results = [] - try: - super()._detect() - reentrancies, warnings = self.get_readonly_reentrancies() - results += self._gen_results(reentrancies, "Readonly-reentrancy in ") - results += self._gen_results( - warnings, - "Potential vulnerable to readonly-reentrancy function (if read in other function)", - ) - except Exception as e: - info = [ - "Error during detection of readonly-reentrancy:\n", - "Please inform this to Yhtyyar\n", - f"error details:", - e, - ] - return results diff --git a/slither_pess/detectors/readonly_reentrancy/reentrancy/__init__.py b/slither_pess/detectors/readonly_reentrancy/reentrancy/__init__.py deleted file mode 100644 index e69de29..0000000 diff --git a/slither_pess/detectors/readonly_reentrancy/reentrancy/reentrancy.py b/slither_pess/detectors/readonly_reentrancy/reentrancy/reentrancy.py deleted file mode 100644 index 5a40ffc..0000000 --- a/slither_pess/detectors/readonly_reentrancy/reentrancy/reentrancy.py +++ /dev/null @@ -1,314 +0,0 @@ -"""" - This is a copy paste from the original slither reentrancy v0.9.1 - - - - Re-entrancy detection - Based on heuristics, it may lead to FP and FN - Iterate over all the nodes of the graph until reaching a fixpoint -""" -from collections import defaultdict -from typing import Set, Dict, Union - -from slither.core.cfg.node import NodeType, Node -from slither.core.declarations import Function -from slither.core.expressions import UnaryOperation, UnaryOperationType -from slither.core.variables.variable import Variable -from slither.detectors.abstract_detector import AbstractDetector -from slither.slithir.operations import Call, EventCall - - -def union_dict(d1, d2): - d3 = { - k: d1.get(k, set()) | d2.get(k, set()) - for k in set(list(d1.keys()) + list(d2.keys())) - } - return defaultdict(set, d3) - - -def dict_are_equal(d1, d2): - if set(list(d1.keys())) != set(list(d2.keys())): - return False - return all(set(d1[k]) == set(d2[k]) for k in d1.keys()) - - -def is_subset( - new_info: Dict[Union[Variable, Node], Set[Node]], - old_info: Dict[Union[Variable, Node], Set[Node]], -): - for k in new_info.keys(): - if k not in old_info: - return False - if not new_info[k].issubset(old_info[k]): - return False - return True - - -def to_hashable(d: Dict[Node, Set[Node]]): - list_tuple = list( - tuple((k, tuple(sorted(values, key=lambda x: x.node_id)))) - for k, values in d.items() - ) - return tuple(sorted(list_tuple, key=lambda x: x[0].node_id)) - - -class AbstractState: - def __init__(self): - # send_eth returns the list of calls sending value - # calls returns the list of calls that can callback - # read returns the variable read - # read_prior_calls returns the variable read prior a call - self._send_eth: Dict[Node, Set[Node]] = defaultdict(set) - self._calls: Dict[Node, Set[Node]] = defaultdict(set) - self._reads: Dict[Variable, Set[Node]] = defaultdict(set) - self._reads_prior_calls: Dict[Node, Set[Variable]] = defaultdict(set) - self._events: Dict[EventCall, Set[Node]] = defaultdict(set) - self._written: Dict[Variable, Set[Node]] = defaultdict(set) - - @property - def send_eth(self) -> Dict[Node, Set[Node]]: - """ - Return the list of calls sending value - :return: - """ - return self._send_eth - - @property - def calls(self) -> Dict[Node, Set[Node]]: - """ - Return the list of calls that can callback - :return: - """ - return self._calls - - @property - def reads(self) -> Dict[Variable, Set[Node]]: - """ - Return of variables that are read - :return: - """ - return self._reads - - @property - def written(self) -> Dict[Variable, Set[Node]]: - """ - Return of variables that are written - :return: - """ - return self._written - - @property - def reads_prior_calls(self) -> Dict[Node, Set[Variable]]: - """ - Return the dictionary node -> variables read before any call - :return: - """ - return self._reads_prior_calls - - @property - def events(self) -> Dict[EventCall, Set[Node]]: - """ - Return the list of events - :return: - """ - return self._events - - def merge_fathers(self, node, skip_father, detector): - for father in node.fathers: - if detector.KEY in father.context: - self._send_eth = union_dict( - self._send_eth, - { - key: values - for key, values in father.context[detector.KEY].send_eth.items() - if key != skip_father - }, - ) - self._calls = union_dict( - self._calls, - { - key: values - for key, values in father.context[detector.KEY].calls.items() - if key != skip_father - }, - ) - self._reads = union_dict( - self._reads, father.context[detector.KEY].reads - ) - self._reads_prior_calls = union_dict( - self.reads_prior_calls, - father.context[detector.KEY].reads_prior_calls, - ) - - def analyze_node(self, node, detector): - state_vars_read: Dict[Variable, Set[Node]] = defaultdict( - set, {v: {node} for v in node.state_variables_read} - ) - - # All the state variables written - state_vars_written: Dict[Variable, Set[Node]] = defaultdict( - set, {v: {node} for v in node.state_variables_written} - ) - slithir_operations = [] - # Add the state variables written in internal calls - for internal_call in node.internal_calls: - # Filter to Function, as internal_call can be a solidity call - if isinstance(internal_call, Function): - for internal_node in internal_call.all_nodes(): - for read in internal_node.state_variables_read: - state_vars_read[read].add(internal_node) - for write in internal_node.state_variables_written: - state_vars_written[write].add(internal_node) - slithir_operations += internal_call.all_slithir_operations() - - contains_call = False - - self._written = state_vars_written - for ir in node.irs + slithir_operations: - if detector.can_callback(ir): - self._calls[node] |= {ir.node} - self._reads_prior_calls[node] = set( - self._reads_prior_calls.get(node, set()) - | set(node.context[detector.KEY].reads.keys()) - | set(state_vars_read.keys()) - ) - contains_call = True - - if detector.can_send_eth(ir): - self._send_eth[node] |= {ir.node} - - if isinstance(ir, EventCall): - self._events[ir] |= {ir.node, node} - - self._reads = union_dict(self._reads, state_vars_read) - - return contains_call - - def add(self, fathers): - self._send_eth = union_dict(self._send_eth, fathers.send_eth) - self._calls = union_dict(self._calls, fathers.calls) - self._reads = union_dict(self._reads, fathers.reads) - self._reads_prior_calls = union_dict( - self._reads_prior_calls, fathers.reads_prior_calls - ) - - def does_not_bring_new_info(self, new_info): - if is_subset(new_info.calls, self.calls): - if is_subset(new_info.send_eth, self.send_eth): - if is_subset(new_info.reads, self.reads): - if dict_are_equal( - new_info.reads_prior_calls, self.reads_prior_calls - ): - return True - return False - - -def _filter_if(node): - """ - Check if the node is a condtional node where - there is an external call checked - Heuristic: - - The call is a IF node - - It contains a, external call - - The condition is the negation (!) - This will work only on naive implementation - """ - return ( - isinstance(node.expression, UnaryOperation) - and node.expression.type == UnaryOperationType.BANG - ) - - -class Reentrancy(AbstractDetector): - KEY = "REENTRANCY" - WIKI_EXPLOIT_SCENARIO = "Check original reentrancy" - WIKI_RECOMMENDATION = "Check original reentrancy" - - # can_callback and can_send_eth are static method - # allowing inherited classes to define different behaviors - # For example reentrancy_no_gas consider Send and Transfer as reentrant functions - @staticmethod - def can_callback(ir): - """ - Detect if the node contains a call that can - be used to re-entrance - Consider as valid target: - - low level call - - high level call - """ - return isinstance(ir, Call) and ir.can_reenter() - - @staticmethod - def can_send_eth(ir): - """ - Detect if the node can send eth - """ - return isinstance(ir, Call) and ir.can_send_eth() - - def _explore(self, node, visited, skip_father=None): - """ - Explore the CFG and look for re-entrancy - Heuristic: There is a re-entrancy if a state variable is written - after an external call - node.context will contains the external calls executed - It contains the calls executed in father nodes - if node.context is not empty, and variables are written, a re-entrancy is possible - """ - if node in visited: - return - - visited = visited + [node] - - fathers_context = AbstractState() - fathers_context.merge_fathers(node, skip_father, self) - - # Exclude path that dont bring further information - if node in self.visited_all_paths: - if self.visited_all_paths[node].does_not_bring_new_info(fathers_context): - return - else: - self.visited_all_paths[node] = AbstractState() - - self.visited_all_paths[node].add(fathers_context) - - node.context[self.KEY] = fathers_context - - contains_call = fathers_context.analyze_node(node, self) - node.context[self.KEY] = fathers_context - - sons = node.sons - if contains_call and node.type in [NodeType.IF, NodeType.IFLOOP]: - if _filter_if(node): - son = sons[0] - self._explore(son, visited, node) - sons = sons[1:] - else: - son = sons[1] - self._explore(son, visited, node) - sons = [sons[0]] - - for son in sons: - self._explore(son, visited) - - def detect_reentrancy(self, contract): - for function in contract.functions_and_modifiers_declared: - if not function.is_constructor: - if function.is_implemented: - if self.KEY in function.context: - continue - self._explore(function.entry_point, []) - function.context[self.KEY] = True - - def _detect(self): - """""" - # if a node was already visited by another path - # we will only explore it if the traversal brings - # new variables written - # This speedup the exploration through a light fixpoint - # Its particular useful on 'complex' functions with several loops and conditions - self.visited_all_paths = {} # pylint: disable=attribute-defined-outside-init - - for c in self.contracts: - self.detect_reentrancy(c) - - return [] diff --git a/slitherin-benchmark b/slitherin-benchmark index e8a7b64..3500000 160000 --- a/slitherin-benchmark +++ b/slitherin-benchmark @@ -1 +1 @@ -Subproject commit e8a7b64b52af91eb257399cdaf16c68c5695d9d7 +Subproject commit 3500000097be15fa8bb5a52fa84c2ca8b11d8bbf diff --git a/slither_pess/detectors/readonly_reentrancy/balancer_readonly_reentrancy.py b/slitherin/detectors/balancer/balancer_readonly_reentrancy.py similarity index 100% rename from slither_pess/detectors/readonly_reentrancy/balancer_readonly_reentrancy.py rename to slitherin/detectors/balancer/balancer_readonly_reentrancy.py From d61369207e99957906508dcb6047948dc5d3fdf9 Mon Sep 17 00:00:00 2001 From: Nikita Kirillov Date: Fri, 22 Mar 2024 14:40:45 +0500 Subject: [PATCH 30/48] add_init_balancer --- slitherin/__init__.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/slitherin/__init__.py b/slitherin/__init__.py index bae8c6c..203ab3a 100644 --- a/slitherin/__init__.py +++ b/slitherin/__init__.py @@ -32,6 +32,7 @@ from slitherin.detectors.arbitrum.arbitrum_chainlink_price_feed import ArbitrumChainlinkPriceFeed from slitherin.detectors.potential_arith_overflow import PotentialArithmOverflow from slitherin.detectors.curve.curve_readonly_reentrancy import CurveReadonlyReentrancy +from slitherin.detectors.balancer.balancer_readonly_reentrancy import BalancerReadonlyReentrancy artbitrum_detectors = [ ArbitrumPrevrandaoDifficulty, @@ -65,6 +66,7 @@ AAVEFlashloanCallbackDetector, PotentialArithmOverflow, CurveReadonlyReentrancy, + BalancerReadonlyReentrancy ] plugin_printers = [] From bee7127a7ccd6b4bb6c7ee40d8762a86a55470ec Mon Sep 17 00:00:00 2001 From: Nikita Kirillov Date: Fri, 22 Mar 2024 14:41:49 +0500 Subject: [PATCH 31/48] package_name update --- package-lock.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package-lock.json b/package-lock.json index dfcf7dc..8fb9792 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,5 +1,5 @@ { - "name": "custom_detectors", + "name": "slitherin", "lockfileVersion": 2, "requires": true, "packages": { From 52aa8ec927cf66d1448d029ccdd1c751456b13d6 Mon Sep 17 00:00:00 2001 From: Nikita Kirillov Date: Fri, 22 Mar 2024 15:00:05 +0500 Subject: [PATCH 32/48] updated_detectors_table --- README.md | 1 + 1 file changed, 1 insertion(+) diff --git a/README.md b/README.md index b15d605..6bdc9ad 100644 --- a/README.md +++ b/README.md @@ -99,6 +99,7 @@ Slitherin detectors are included into original Slither after the installation. Y | [Arbitrary Call](https://github.com/pessimistic-io/slitherin/blob/master/slitherin/detectors/arbitrary_call/arbitrary_call.py) | [Explore](https://github.com/pessimistic-io/slitherin/blob/master/docs/arbitrary_call.md) | [Test](https://github.com/pessimistic-io/slitherin/blob/master/tests/arbitrary_call_test.sol) | 0 | | [Elliptic Curve Recover](https://github.com/pessimistic-io/slitherin/blob/master/slitherin/detectors/ecrecover.py) | [Explore](https://github.com/pessimistic-io/slitherin/blob/master/docs/ecrecover.md) | [Test](https://github.com/pessimistic-io/slitherin/blob/master/tests/ecrecover.sol) | 0 | | [Public vs External](https://github.com/pessimistic-io/slitherin/blob/master/slitherin/detectors/public_vs_external.py) | [Explore](https://github.com/pessimistic-io/slitherin/blob/master/docs/public_vs_external.md) | [Test](https://github.com/pessimistic-io/slitherin/blob/master/tests/public_vs_external_test.sol) | 0 | +| [Balancer Read-only Reentrancy](https://github.com/pessimistic-io/slitherin/blob/master/slitherin/detectors/balancer/balancer_readonly_reentrancy.py) | [Explore](https://github.com/pessimistic-io/slitherin/blob/master/docs/balancer/readonly_reentrancy.md) | [Test](https://github.com/pessimistic-io/slitherin/blob/master/tests/balancer/readonly_reentrancy_test.sol) | 0 | **Please note:** From 539e74694494c634c00c908f92599ce013107ce2 Mon Sep 17 00:00:00 2001 From: Joran Honig Date: Thu, 22 Feb 2024 11:15:29 -0600 Subject: [PATCH 33/48] remove relative imports --- .../arbitrum_prevrandao_difficulty.py | 2 +- .../arbitrum/block_number_timestamp.py | 2 +- .../detectors/arbitrum/solidity_version.py | 61 ------------------- slitherin/detectors/read_only_reentrancy.py | 2 +- 4 files changed, 3 insertions(+), 64 deletions(-) delete mode 100644 slitherin/detectors/arbitrum/solidity_version.py diff --git a/slitherin/detectors/arbitrum/arbitrum_prevrandao_difficulty.py b/slitherin/detectors/arbitrum/arbitrum_prevrandao_difficulty.py index b81b497..9e8b243 100644 --- a/slitherin/detectors/arbitrum/arbitrum_prevrandao_difficulty.py +++ b/slitherin/detectors/arbitrum/arbitrum_prevrandao_difficulty.py @@ -12,7 +12,7 @@ SolidityFunction, ) -from ...consts import ARBITRUM_KEY +from slitherin.consts import ARBITRUM_KEY class ArbitrumPrevrandaoDifficulty(AbstractDetector): diff --git a/slitherin/detectors/arbitrum/block_number_timestamp.py b/slitherin/detectors/arbitrum/block_number_timestamp.py index f142f1c..b43b58b 100644 --- a/slitherin/detectors/arbitrum/block_number_timestamp.py +++ b/slitherin/detectors/arbitrum/block_number_timestamp.py @@ -12,7 +12,7 @@ SolidityFunction, ) -from ...consts import ARBITRUM_KEY +from slitherin.consts import ARBITRUM_KEY class ArbitrumBlockNumberTimestamp(AbstractDetector): diff --git a/slitherin/detectors/arbitrum/solidity_version.py b/slitherin/detectors/arbitrum/solidity_version.py deleted file mode 100644 index 34a2def..0000000 --- a/slitherin/detectors/arbitrum/solidity_version.py +++ /dev/null @@ -1,61 +0,0 @@ -import os -from typing import List -from slither.utils.output import Output -from slither.core.cfg.node import Node -from slither.detectors.abstract_detector import ( - AbstractDetector, - DetectorClassification, - make_solc_versions, -) -from slither.core.declarations import Function -from slither.slithir.operations.event_call import EventCall -from slither.analyses.data_dependency.data_dependency import is_dependent -from slither.slithir.operations import SolidityCall -from slither.core.declarations.solidity_variables import ( - SolidityVariableComposed, - SolidityFunction, -) - -from ...consts import ARBITRUM_KEY - - -class ArbitrumSolidityVersion(AbstractDetector): - """ - Checks that sol version >= 0.8.20 is not used inside an Arbitrum contract - """ - - ARGUMENT = "pess-arb-solidity-version" # slither will launch the detector with slither.py --detect mydetector - HELP = "sol version >= 0.8.20 is used in contract that will be deployed to Arbitrum (Potential usage of PUSH0, which is not supported)" - IMPACT = DetectorClassification.HIGH - CONFIDENCE = DetectorClassification.LOW - - WIKI = "https://github.com/pessimistic-io/slitherin/blob/master/docs/arb_difficulty_randao.md" - WIKI_TITLE = "Arbitrum solidity version" - WIKI_DESCRIPTION = "Potential usage of PUSH0 opcode" - WIKI_EXPLOIT_SCENARIO = "N/A" - WIKI_RECOMMENDATION = ( - "Either, use versions 0.8.19 and below, or EVM versions below shanghai" - ) - - def _detect(self) -> List[Output]: - """Main function""" - results = [] - - if os.getenv(ARBITRUM_KEY) is None: - return results - - if ( - self.compilation_unit.is_solidity - and self.compilation_unit.solc_version in make_solc_versions(8, 20, 99) - ): - results.append( - self.generate_result( - [ - "Potential usage of solidity version >= 0.8.20 detected. Solidity in these versions will utilize PUSH0 opcode, ", - "which is not supported on Arbitrum. ", - "Either, use versions 0.8.19 and below, or EVM versions below shanghai", - ] - ) - ) - - return results diff --git a/slitherin/detectors/read_only_reentrancy.py b/slitherin/detectors/read_only_reentrancy.py index 7b17c61..2bd20ae 100644 --- a/slitherin/detectors/read_only_reentrancy.py +++ b/slitherin/detectors/read_only_reentrancy.py @@ -9,7 +9,7 @@ from slither.core.declarations import Function from slither.core.cfg.node import NodeType, Node, Contract from slither.detectors.abstract_detector import DetectorClassification -from .reentrancy.reentrancy import ( +from slitherin.detectors.reentrancy.reentrancy import ( Reentrancy, to_hashable, AbstractState, From 76d0afabff7a78909e6146888ec2b6f1ef9646fb Mon Sep 17 00:00:00 2001 From: Joran Honig Date: Thu, 22 Feb 2024 11:15:46 -0600 Subject: [PATCH 34/48] update napalm entry point to restrict inclusions --- slitherin/napalm.py | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/slitherin/napalm.py b/slitherin/napalm.py index 3e10b37..5e60d04 100644 --- a/slitherin/napalm.py +++ b/slitherin/napalm.py @@ -1,5 +1,5 @@ import napalm.package -import slitherin.detectors +import slitherin as slitherin def entry_point(): @@ -9,4 +9,10 @@ def entry_point(): It returns a dictionary of Collections, keyed by the name of the collection. """ - return napalm.package.entry_point(slitherin.detectors) + _include = ("detectors",) + + return [ + collection + for collection in napalm.package.entry_point(slitherin) + if collection.collection_name in _include + ] From f9c8ac5f1fcd37d3d02dbddf6fd9d17f4a2d1a4a Mon Sep 17 00:00:00 2001 From: Joran Honig Date: Thu, 22 Feb 2024 11:19:30 -0600 Subject: [PATCH 35/48] revert undesired changes to setup --- setup.py | 2 + .../detectors/arbitrum/solidity_version.py | 61 +++++++++++++++++++ 2 files changed, 63 insertions(+) create mode 100644 slitherin/detectors/arbitrum/solidity_version.py diff --git a/setup.py b/setup.py index c276088..dd011a6 100644 --- a/setup.py +++ b/setup.py @@ -20,8 +20,10 @@ "Topic :: Software Development :: Libraries", ], version=SLITHERIN_VERSION, + packages=find_packages(), license="AGPL-3.0", python_requires=">=3.8", + install_requires=["slither-analyzer>=0.10.0"], extras_requires={ "dev": ["twine>=4.0.2"], }, diff --git a/slitherin/detectors/arbitrum/solidity_version.py b/slitherin/detectors/arbitrum/solidity_version.py new file mode 100644 index 0000000..34a2def --- /dev/null +++ b/slitherin/detectors/arbitrum/solidity_version.py @@ -0,0 +1,61 @@ +import os +from typing import List +from slither.utils.output import Output +from slither.core.cfg.node import Node +from slither.detectors.abstract_detector import ( + AbstractDetector, + DetectorClassification, + make_solc_versions, +) +from slither.core.declarations import Function +from slither.slithir.operations.event_call import EventCall +from slither.analyses.data_dependency.data_dependency import is_dependent +from slither.slithir.operations import SolidityCall +from slither.core.declarations.solidity_variables import ( + SolidityVariableComposed, + SolidityFunction, +) + +from ...consts import ARBITRUM_KEY + + +class ArbitrumSolidityVersion(AbstractDetector): + """ + Checks that sol version >= 0.8.20 is not used inside an Arbitrum contract + """ + + ARGUMENT = "pess-arb-solidity-version" # slither will launch the detector with slither.py --detect mydetector + HELP = "sol version >= 0.8.20 is used in contract that will be deployed to Arbitrum (Potential usage of PUSH0, which is not supported)" + IMPACT = DetectorClassification.HIGH + CONFIDENCE = DetectorClassification.LOW + + WIKI = "https://github.com/pessimistic-io/slitherin/blob/master/docs/arb_difficulty_randao.md" + WIKI_TITLE = "Arbitrum solidity version" + WIKI_DESCRIPTION = "Potential usage of PUSH0 opcode" + WIKI_EXPLOIT_SCENARIO = "N/A" + WIKI_RECOMMENDATION = ( + "Either, use versions 0.8.19 and below, or EVM versions below shanghai" + ) + + def _detect(self) -> List[Output]: + """Main function""" + results = [] + + if os.getenv(ARBITRUM_KEY) is None: + return results + + if ( + self.compilation_unit.is_solidity + and self.compilation_unit.solc_version in make_solc_versions(8, 20, 99) + ): + results.append( + self.generate_result( + [ + "Potential usage of solidity version >= 0.8.20 detected. Solidity in these versions will utilize PUSH0 opcode, ", + "which is not supported on Arbitrum. ", + "Either, use versions 0.8.19 and below, or EVM versions below shanghai", + ] + ) + ) + + return results From 3581c9a622db484d6ac65d0ef81282020d483d47 Mon Sep 17 00:00:00 2001 From: Yhtyyar Sahatov Date: Mon, 26 Feb 2024 14:32:09 +0300 Subject: [PATCH 36/48] removed arbitrum solidity version detector --- slitherin/__init__.py | 2 - .../detectors/arbitrum/solidity_version.py | 61 ------------------- tests/arbitrum_solidity_version_test.sol | 4 -- 3 files changed, 67 deletions(-) delete mode 100644 slitherin/detectors/arbitrum/solidity_version.py delete mode 100644 tests/arbitrum_solidity_version_test.sol diff --git a/slitherin/__init__.py b/slitherin/__init__.py index 35c1218..cd9e78b 100644 --- a/slitherin/__init__.py +++ b/slitherin/__init__.py @@ -26,7 +26,6 @@ from slitherin.detectors.arbitrum.arbitrum_prevrandao_difficulty import ( ArbitrumPrevrandaoDifficulty, ) -from slitherin.detectors.arbitrum.solidity_version import ArbitrumSolidityVersion from slitherin.detectors.arbitrum.block_number_timestamp import ( ArbitrumBlockNumberTimestamp, ) @@ -34,7 +33,6 @@ artbitrum_detectors = [ ArbitrumPrevrandaoDifficulty, - ArbitrumSolidityVersion, ArbitrumBlockNumberTimestamp, ] diff --git a/slitherin/detectors/arbitrum/solidity_version.py b/slitherin/detectors/arbitrum/solidity_version.py deleted file mode 100644 index 34a2def..0000000 --- a/slitherin/detectors/arbitrum/solidity_version.py +++ /dev/null @@ -1,61 +0,0 @@ -import os -from typing import List -from slither.utils.output import Output -from slither.core.cfg.node import Node -from slither.detectors.abstract_detector import ( - AbstractDetector, - DetectorClassification, - make_solc_versions, -) -from slither.core.declarations import Function -from slither.slithir.operations.event_call import EventCall -from slither.analyses.data_dependency.data_dependency import is_dependent -from slither.slithir.operations import SolidityCall -from slither.core.declarations.solidity_variables import ( - SolidityVariableComposed, - SolidityFunction, -) - -from ...consts import ARBITRUM_KEY - - -class ArbitrumSolidityVersion(AbstractDetector): - """ - Checks that sol version >= 0.8.20 is not used inside an Arbitrum contract - """ - - ARGUMENT = "pess-arb-solidity-version" # slither will launch the detector with slither.py --detect mydetector - HELP = "sol version >= 0.8.20 is used in contract that will be deployed to Arbitrum (Potential usage of PUSH0, which is not supported)" - IMPACT = DetectorClassification.HIGH - CONFIDENCE = DetectorClassification.LOW - - WIKI = "https://github.com/pessimistic-io/slitherin/blob/master/docs/arb_difficulty_randao.md" - WIKI_TITLE = "Arbitrum solidity version" - WIKI_DESCRIPTION = "Potential usage of PUSH0 opcode" - WIKI_EXPLOIT_SCENARIO = "N/A" - WIKI_RECOMMENDATION = ( - "Either, use versions 0.8.19 and below, or EVM versions below shanghai" - ) - - def _detect(self) -> List[Output]: - """Main function""" - results = [] - - if os.getenv(ARBITRUM_KEY) is None: - return results - - if ( - self.compilation_unit.is_solidity - and self.compilation_unit.solc_version in make_solc_versions(8, 20, 99) - ): - results.append( - self.generate_result( - [ - "Potential usage of solidity version >= 0.8.20 detected. Solidity in these versions will utilize PUSH0 opcode, ", - "which is not supported on Arbitrum. ", - "Either, use versions 0.8.19 and below, or EVM versions below shanghai", - ] - ) - ) - - return results diff --git a/tests/arbitrum_solidity_version_test.sol b/tests/arbitrum_solidity_version_test.sol deleted file mode 100644 index fbe7da6..0000000 --- a/tests/arbitrum_solidity_version_test.sol +++ /dev/null @@ -1,4 +0,0 @@ -// SPDX-License-Identifier: SEE LICENSE IN LICENSE -pragma solidity 0.8.21; - -contract Test {} From d9a684c606e4fb66d95c9e4eeafb4f5784d4d835 Mon Sep 17 00:00:00 2001 From: olegggatttor Date: Fri, 12 Apr 2024 17:50:53 +0300 Subject: [PATCH 37/48] add recursive check for base constructors | strange setter --- slitherin/detectors/strange_setter.py | 10 +++++++++- tests/strange_setter_test.sol | 4 ++++ 2 files changed, 13 insertions(+), 1 deletion(-) diff --git a/slitherin/detectors/strange_setter.py b/slitherin/detectors/strange_setter.py index 323dc9b..2782caa 100644 --- a/slitherin/detectors/strange_setter.py +++ b/slitherin/detectors/strange_setter.py @@ -3,6 +3,7 @@ from slither.detectors.abstract_detector import AbstractDetector, DetectorClassification from slither.core.declarations import Function import slither.core.expressions.new_array as na +from slither.analyses.data_dependency.data_dependency import is_dependent class StrangeSetter(AbstractDetector): @@ -54,11 +55,18 @@ def _is_strange_setter(self, fun: Function) -> bool: for arg in [*external.arguments, external._called._expression]: if str(arg) == str(param): used_params.add(param) + if fun.name == "constructor": + for base_call in fun.explicit_base_constructor_calls: + if not self._is_strange_constructor(base_call): + for param_cur in fun.parameters: + for param_base in base_call.parameters: + if is_dependent(param_base, param_cur, base_call): + used_params.add(param_cur) intersection_len = len(set(fun.parameters) & used_params) return intersection_len != len(fun.parameters) def _is_strange_constructor(self, fun: Function) -> bool: - """Checks if constructor sets nothing""" + """Checks if constructor setqs nothing""" return self._is_strange_setter(fun) def _detect(self) -> List[Output]: diff --git a/tests/strange_setter_test.sol b/tests/strange_setter_test.sol index d2475c3..2c52806 100644 --- a/tests/strange_setter_test.sol +++ b/tests/strange_setter_test.sol @@ -87,3 +87,7 @@ contract OkConstructor { init = true; } } + +contract TestInheritance is StrangeSetter{ + constructor(uint256 _toSet) StrangeSetter(_toSet) {} +} \ No newline at end of file From 93f49f2506355b053065180d678f9607b573a53c Mon Sep 17 00:00:00 2001 From: olegggatttor Date: Fri, 12 Apr 2024 17:51:27 +0300 Subject: [PATCH 38/48] typo --- slitherin/detectors/strange_setter.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/slitherin/detectors/strange_setter.py b/slitherin/detectors/strange_setter.py index 2782caa..44332fb 100644 --- a/slitherin/detectors/strange_setter.py +++ b/slitherin/detectors/strange_setter.py @@ -66,7 +66,7 @@ def _is_strange_setter(self, fun: Function) -> bool: return intersection_len != len(fun.parameters) def _is_strange_constructor(self, fun: Function) -> bool: - """Checks if constructor setqs nothing""" + """Checks if constructor sets nothing""" return self._is_strange_setter(fun) def _detect(self) -> List[Output]: From a2862f328429b61cfdb0e516b2de8acf43f6a963 Mon Sep 17 00:00:00 2001 From: shortdoom Date: Fri, 12 Apr 2024 18:05:00 +0200 Subject: [PATCH 39/48] fix: PotentialArithmOverflow detector fail --- slitherin/detectors/potential_arith_overflow.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/slitherin/detectors/potential_arith_overflow.py b/slitherin/detectors/potential_arith_overflow.py index 61ce44c..c6a1f53 100644 --- a/slitherin/detectors/potential_arith_overflow.py +++ b/slitherin/detectors/potential_arith_overflow.py @@ -86,7 +86,7 @@ def _find_vulnerable_expressions(self, fun: Function) -> list: errors.extend(response_errors) if has_problems: final_results.append((node, str(irs[-1].lvalue._type), errors)) - if len(irs) > 0 and isinstance(irs[-1], ops.Return) and len(fun.return_type) == 1 and str(fun.return_type[0]) in INT_TYPES: # @todo currently works only with single returns + if len(irs) > 0 and isinstance(irs[-1], ops.Return) and fun.return_type is not None and len(fun.return_type) == 1 and str(fun.return_type[0]) in INT_TYPES: # @todo currently works only with single returns expected_bits_cut = str(fun.return_type[0]).removeprefix("uint").removeprefix("int") expected_bits = int(256 if not expected_bits_cut else expected_bits_cut) has_problems = False @@ -115,4 +115,4 @@ def _detect(self) -> List[Output]: for (op, op_ret_type) in op_with_ret_type: info += ["\t\t`", str(op), f"` returns {op_ret_type}, but the type of the resulting expression is {node_final_type}.", "\n"] res.append(self.generate_result(info)) - return res + return res \ No newline at end of file From 3870d212d16b477edcc6ba4603ffe0b481fad80a Mon Sep 17 00:00:00 2001 From: olegggatttor Date: Fri, 12 Apr 2024 21:16:58 +0300 Subject: [PATCH 40/48] fix starge setter fail on new contract creations --- slitherin/detectors/strange_setter.py | 3 +++ tests/strange_setter_test.sol | 6 ++++++ 2 files changed, 9 insertions(+) diff --git a/slitherin/detectors/strange_setter.py b/slitherin/detectors/strange_setter.py index 44332fb..62c7a02 100644 --- a/slitherin/detectors/strange_setter.py +++ b/slitherin/detectors/strange_setter.py @@ -3,6 +3,7 @@ from slither.detectors.abstract_detector import AbstractDetector, DetectorClassification from slither.core.declarations import Function import slither.core.expressions.new_array as na +import slither.core.expressions.new_contract as nc from slither.analyses.data_dependency.data_dependency import is_dependent @@ -52,6 +53,8 @@ def _is_strange_setter(self, fun: Function) -> bool: for external in fun.external_calls_as_expressions: if isinstance(external._called, na.NewArray): continue + if isinstance(external._called, nc.NewContract): # skip new contract calls, idk how to get arguments passed to creation + continue for arg in [*external.arguments, external._called._expression]: if str(arg) == str(param): used_params.add(param) diff --git a/tests/strange_setter_test.sol b/tests/strange_setter_test.sol index 2c52806..58108ed 100644 --- a/tests/strange_setter_test.sol +++ b/tests/strange_setter_test.sol @@ -90,4 +90,10 @@ contract OkConstructor { contract TestInheritance is StrangeSetter{ constructor(uint256 _toSet) StrangeSetter(_toSet) {} +} + +contract TestNewContract { + constructor(uint256 _toSet) { + new TestInheritance(_toSet); + } } \ No newline at end of file From fdec81b1816fe77cfcb72ddb5e83b4a81914bb90 Mon Sep 17 00:00:00 2001 From: olegggatttor Date: Tue, 16 Apr 2024 14:27:04 +0300 Subject: [PATCH 41/48] add vyper version detector --- docs/curve_vyper_reentrancy.md | 26 +++++++++++ slitherin/__init__.py | 2 + .../vyper/reentrancy_curve_vyper_version.py | 44 +++++++++++++++++++ tests/vyper/curve_vyper_reentrancy_test.vy | 15 +++++++ 4 files changed, 87 insertions(+) create mode 100644 docs/curve_vyper_reentrancy.md create mode 100644 slitherin/detectors/vyper/reentrancy_curve_vyper_version.py create mode 100644 tests/vyper/curve_vyper_reentrancy_test.vy diff --git a/docs/curve_vyper_reentrancy.md b/docs/curve_vyper_reentrancy.md new file mode 100644 index 0000000..c679f16 --- /dev/null +++ b/docs/curve_vyper_reentrancy.md @@ -0,0 +1,26 @@ +# Curve Readonly Reentrancy + +## Configuration + +- Check: `pess-curve-vyper-reentrancy` +- Severity: `High` +- Confidence: `High` + +## Description + +Finds if the code is compiled with vulnerable Vyper compiler version and contains non-reentrant modifiers. +Details: +- [Curve exploit postmortem](https://hackmd.io/@LlamaRisk/BJzSKHNjn) +- [Postmortem from Vyper team](https://hackmd.io/@vyperlang/HJUgNMhs2) + +## Vulnerable Scenario + +[test scenarios](../../tests/vyper/curve_vyper_reentrancy_test.vy) + +## Related Attacks + +- [Vyper compiler exploits](https://www.halborn.com/blog/post/explained-the-vyper-bug-hack-july-2023) + +## Recomendations + +- Upgrade the version of your Vyper compiler. \ No newline at end of file diff --git a/slitherin/__init__.py b/slitherin/__init__.py index bae8c6c..b6dc284 100644 --- a/slitherin/__init__.py +++ b/slitherin/__init__.py @@ -32,6 +32,7 @@ from slitherin.detectors.arbitrum.arbitrum_chainlink_price_feed import ArbitrumChainlinkPriceFeed from slitherin.detectors.potential_arith_overflow import PotentialArithmOverflow from slitherin.detectors.curve.curve_readonly_reentrancy import CurveReadonlyReentrancy +from slitherin.detectors.vyper.reentrancy_curve_vyper_version import CurveVyperReentrancy artbitrum_detectors = [ ArbitrumPrevrandaoDifficulty, @@ -65,6 +66,7 @@ AAVEFlashloanCallbackDetector, PotentialArithmOverflow, CurveReadonlyReentrancy, + CurveVyperReentrancy ] plugin_printers = [] diff --git a/slitherin/detectors/vyper/reentrancy_curve_vyper_version.py b/slitherin/detectors/vyper/reentrancy_curve_vyper_version.py new file mode 100644 index 0000000..db848ea --- /dev/null +++ b/slitherin/detectors/vyper/reentrancy_curve_vyper_version.py @@ -0,0 +1,44 @@ +from typing import List +from slither.utils.output import Output +from slither.detectors.abstract_detector import AbstractDetector, DetectorClassification +from slither.core.declarations import Function +from slither.slithir.operations.event_call import EventCall + +VULNERABLE_VERSIONS = ['0.2.15', '0.2.16', '0.3.0'] +class CurveVyperReentrancy(AbstractDetector): + """ + Sees if contract setters do not emit events + """ + + ARGUMENT = 'pess-curve-vyper-reentrancy' # slither will launch the detector with slither.py --detect mydetector + HELP = f'Vyper compiler versions {", ".join(VULNERABLE_VERSIONS)} are vulnerable to malfunctioning re-entrancy guards. Upgrade your compiler version.' + IMPACT = DetectorClassification.HIGH + CONFIDENCE = DetectorClassification.HIGH + + WIKI = 'https://github.com/pessimistic-io/slitherin/blob/master/docs/curve_vyper_reentrancy.md' + WIKI_TITLE = 'Vulnerable Vyper version' + WIKI_DESCRIPTION = "Some Vyper versions are vulnerable to malfunctioning re-entrancy guards." + WIKI_EXPLOIT_SCENARIO = 'https://hackmd.io/@LlamaRisk/BJzSKHNjn' + WIKI_RECOMMENDATION = 'Upgrade the version of your Vyper compiler.' + + def _detect(self) -> List[Output]: + res = [] + if not self.compilation_unit.is_vyper: + return res + compiler_version = self.compilation_unit.compiler_version.version + if compiler_version not in VULNERABLE_VERSIONS: + return res + + for contract in self.contracts: + for f in contract.functions_entry_points: + for modifier in f.modifiers: + if modifier.name.startswith("nonreentrant"): + res += ["\t", modifier.name, " in ", f, " function.\n"] + if not res: + return res + return [self.generate_result( + [f"Vyper {compiler_version} compiler version is vulnerable to malfunctioning re-entrancy guards. Found vulnerable usages:\n", + *[x for x in res], + "\n" + ] + )] diff --git a/tests/vyper/curve_vyper_reentrancy_test.vy b/tests/vyper/curve_vyper_reentrancy_test.vy new file mode 100644 index 0000000..6d8fcb1 --- /dev/null +++ b/tests/vyper/curve_vyper_reentrancy_test.vy @@ -0,0 +1,15 @@ +# @version =0.2.15 + +@external +@nonreentrant("hello_lock") +def helloWorld() -> String[24]: + return "Hello World!" + +@external +@nonreentrant("another_lock") +def another_reentrant_func() -> uint256: + return 1 + +@external +def normal_func() -> uint256: + return 0 \ No newline at end of file From 7669e6796247310310c49b054db1137bc174fa4e Mon Sep 17 00:00:00 2001 From: olegggatttor Date: Tue, 16 Apr 2024 14:35:09 +0300 Subject: [PATCH 42/48] add file --- slitherin/detectors/price_manipulation.py | 37 +++++++++++++++++++++++ 1 file changed, 37 insertions(+) create mode 100644 slitherin/detectors/price_manipulation.py diff --git a/slitherin/detectors/price_manipulation.py b/slitherin/detectors/price_manipulation.py new file mode 100644 index 0000000..155b8d8 --- /dev/null +++ b/slitherin/detectors/price_manipulation.py @@ -0,0 +1,37 @@ +from typing import List +from slither.utils.output import Output +from slither.detectors.abstract_detector import AbstractDetector, DetectorClassification +from slither.core.declarations import Function +from slither.slithir.operations.event_call import EventCall + + +class PriceManipulationDetector(AbstractDetector): + """ + Sees if contract setters do not emit events + """ + + ARGUMENT = 'pess-price-manipulation' # slither will launch the detector with slither.py --detect mydetector + HELP = 'Contract function does not emit event after the value is set' + IMPACT = DetectorClassification.LOW + CONFIDENCE = DetectorClassification.MEDIUM + + WIKI = 'https://github.com/pessimistic-io/slitherin/blob/master/docs/event_setter.md' + WIKI_TITLE = 'Missing Event Setter' + WIKI_DESCRIPTION = "Setter-functions must emit events" + WIKI_EXPLOIT_SCENARIO = 'N/A' + WIKI_RECOMMENDATION = 'Emit events in setter functions' + + def _detect(self) -> List[Output]: + """Main function""" + res = [] + for contract in self.compilation_unit.contracts_derived: + if not contract.is_interface: + for f in contract.functions_and_modifiers_declared: + if f.name.startswith("set"): + x = self._emits_event(f) + if not x: + res.append(self.generate_result([ + "Setter function ", + f, ' does not emit an event' + '\n'])) + return res From 9650c1850a5cba9439155e6cd86723b2c62db81b Mon Sep 17 00:00:00 2001 From: olegggatttor Date: Tue, 16 Apr 2024 14:36:01 +0300 Subject: [PATCH 43/48] rm comment --- slitherin/detectors/vyper/reentrancy_curve_vyper_version.py | 4 ---- 1 file changed, 4 deletions(-) diff --git a/slitherin/detectors/vyper/reentrancy_curve_vyper_version.py b/slitherin/detectors/vyper/reentrancy_curve_vyper_version.py index db848ea..34c3ae5 100644 --- a/slitherin/detectors/vyper/reentrancy_curve_vyper_version.py +++ b/slitherin/detectors/vyper/reentrancy_curve_vyper_version.py @@ -6,10 +6,6 @@ VULNERABLE_VERSIONS = ['0.2.15', '0.2.16', '0.3.0'] class CurveVyperReentrancy(AbstractDetector): - """ - Sees if contract setters do not emit events - """ - ARGUMENT = 'pess-curve-vyper-reentrancy' # slither will launch the detector with slither.py --detect mydetector HELP = f'Vyper compiler versions {", ".join(VULNERABLE_VERSIONS)} are vulnerable to malfunctioning re-entrancy guards. Upgrade your compiler version.' IMPACT = DetectorClassification.HIGH From 9028442a6480900d8c965696526cc0007ed3d2e1 Mon Sep 17 00:00:00 2001 From: olegggatttor Date: Tue, 16 Apr 2024 16:14:55 +0300 Subject: [PATCH 44/48] add detector --- docs/price_manipulation.md | 15 +++++ slitherin/__init__.py | 2 + slitherin/detectors/price_manipulation.py | 78 ++++++++++++++++------- tests/price_manipulation_test.sol | 43 +++++++++++++ 4 files changed, 115 insertions(+), 23 deletions(-) create mode 100644 docs/price_manipulation.md create mode 100644 tests/price_manipulation_test.sol diff --git a/docs/price_manipulation.md b/docs/price_manipulation.md new file mode 100644 index 0000000..50b3742 --- /dev/null +++ b/docs/price_manipulation.md @@ -0,0 +1,15 @@ +# Price Manipulation through token transfers + +## Configuration +* Check: `pess-price-manipulation` +* Severity: `High` +* Confidence: `Low` + +## Description +The detector finds calculations that depend on the balance and supply of some token. Such calculations could be manipulated through direct transfers to the contract, increasing its balance. + +## Vulnerable Scenario +[test scenario](../tests/price_manipulation_test.sol) + +## Recommendation +Avoid possible manipulations of calculations because of external transfers. \ No newline at end of file diff --git a/slitherin/__init__.py b/slitherin/__init__.py index bae8c6c..8b3b5a9 100644 --- a/slitherin/__init__.py +++ b/slitherin/__init__.py @@ -32,6 +32,7 @@ from slitherin.detectors.arbitrum.arbitrum_chainlink_price_feed import ArbitrumChainlinkPriceFeed from slitherin.detectors.potential_arith_overflow import PotentialArithmOverflow from slitherin.detectors.curve.curve_readonly_reentrancy import CurveReadonlyReentrancy +from slitherin.detectors.price_manipulation import PriceManipulationDetector artbitrum_detectors = [ ArbitrumPrevrandaoDifficulty, @@ -65,6 +66,7 @@ AAVEFlashloanCallbackDetector, PotentialArithmOverflow, CurveReadonlyReentrancy, + PriceManipulationDetector ] plugin_printers = [] diff --git a/slitherin/detectors/price_manipulation.py b/slitherin/detectors/price_manipulation.py index 155b8d8..59ebd87 100644 --- a/slitherin/detectors/price_manipulation.py +++ b/slitherin/detectors/price_manipulation.py @@ -3,35 +3,67 @@ from slither.detectors.abstract_detector import AbstractDetector, DetectorClassification from slither.core.declarations import Function from slither.slithir.operations.event_call import EventCall +from slither.slithir.operations.high_level_call import HighLevelCall +from slither.slithir.operations.internal_call import InternalCall +from slither.slithir.operations.binary import Binary, BinaryType +from slither.analyses.data_dependency.data_dependency import is_dependent class PriceManipulationDetector(AbstractDetector): - """ - Sees if contract setters do not emit events - """ - ARGUMENT = 'pess-price-manipulation' # slither will launch the detector with slither.py --detect mydetector - HELP = 'Contract function does not emit event after the value is set' - IMPACT = DetectorClassification.LOW - CONFIDENCE = DetectorClassification.MEDIUM + HELP = 'Contract math can be manipulated through external token transfers.' + IMPACT = DetectorClassification.HIGH + CONFIDENCE = DetectorClassification.LOW - WIKI = 'https://github.com/pessimistic-io/slitherin/blob/master/docs/event_setter.md' - WIKI_TITLE = 'Missing Event Setter' - WIKI_DESCRIPTION = "Setter-functions must emit events" + WIKI = 'https://github.com/pessimistic-io/slitherin/blob/master/docs/price_manipulation.md' + WIKI_TITLE = '# Price Manipulation through token transfers' + WIKI_DESCRIPTION = "Calculations could be manipulated through direct transfers to the contract, increasing its balance as they depends on these balances." WIKI_EXPLOIT_SCENARIO = 'N/A' - WIKI_RECOMMENDATION = 'Emit events in setter functions' + WIKI_RECOMMENDATION = 'Avoid possible manipulations of calculations because of external transfers.' def _detect(self) -> List[Output]: - """Main function""" - res = [] - for contract in self.compilation_unit.contracts_derived: + all_balance_vars = [] + all_supply_vars = [] + all_binary_ops = [] + for contract in self.contracts: if not contract.is_interface: - for f in contract.functions_and_modifiers_declared: - if f.name.startswith("set"): - x = self._emits_event(f) - if not x: - res.append(self.generate_result([ - "Setter function ", - f, ' does not emit an event' - '\n'])) - return res + for func in contract.functions: + for n in func.nodes: + for x in n.irs: + if isinstance(x, HighLevelCall): + if str(x.function_name).lower() == "balanceof": + all_balance_vars.append((n, x._lvalue)) + if "supply" in str(x.function_name).lower(): + all_supply_vars.append((n, x._lvalue)) + if isinstance(x, InternalCall): + if "supply" in str(x.function_name).lower(): + all_supply_vars.append((n, x._lvalue)) + if isinstance(x, Binary): + all_binary_ops.append((n, x)) + results = [] + for (balance_node, bal) in all_balance_vars: + for (supply_node, supply) in all_supply_vars: + for (node, bin_op) in all_binary_ops: + l, r = bin_op.get_variable + is_bal_dependent = is_dependent(l, bal, contract) + is_supply_dependent = is_dependent(r, supply, contract) + if is_bal_dependent and is_supply_dependent: + results.append((node, balance_node, supply_node)) + if not results: + return [] + + + response = [] + for issue_node, balance_node, supply_node in results: + res = [] + res.append("Calculation ") + res.append(issue_node) + res.append(" depends on balance and token supply - these values could be manipulated through external calls.\n") + res.append("\tBalance dependency: ") + res.append(balance_node) + res.append("\n") + res.append("\tSupply dependency: ") + res.append(supply_node) + res.append("\n") + response.append(self.generate_result(res)) + return response diff --git a/tests/price_manipulation_test.sol b/tests/price_manipulation_test.sol new file mode 100644 index 0000000..4963514 --- /dev/null +++ b/tests/price_manipulation_test.sol @@ -0,0 +1,43 @@ +interface IERC20 { + function totalSupply() external view returns (uint256); + function balanceOf(address account) external view returns (uint256); +} +contract Test1 { + IERC20 token; + + function test_vuln_1() external returns(uint256 price) { + price = token.balanceOf(address(this)) / token.totalSupply(); + } + + function test_vuln_2() external returns(uint256 price) { + uint256 bal = token.balanceOf(address(this)); + uint256 supply = token.totalSupply(); + price = bal / supply; + } + + function test_vuln_3() external returns(uint256 price) { + uint256 bal = getBalance(); + price = bal / token.totalSupply(); + } + + function test_vuln_4() external returns(uint256 price) { + uint256 bal = getBalance(); + price = 10 + (bal / (token.totalSupply() * 5)); + } + + function test_vuln_5() external returns(uint256 price) { + price = getBalance() / mySupply(); + } + + function test_vuln_6() external returns(uint256 price) { + price = getBalance() + mySupply() + 1; + } + + function getBalance() public returns(uint256 bal) { + bal = token.balanceOf(msg.sender); + } + + function mySupply() public returns (uint256) { + return 100; + } +} \ No newline at end of file From a96c304e0c14dd06216882d7b2e4c2d288679b51 Mon Sep 17 00:00:00 2001 From: olegggatttor Date: Tue, 16 Apr 2024 17:14:33 +0300 Subject: [PATCH 45/48] add native support --- slitherin/detectors/price_manipulation.py | 8 +++++--- tests/price_manipulation_test.sol | 4 ++++ 2 files changed, 9 insertions(+), 3 deletions(-) diff --git a/slitherin/detectors/price_manipulation.py b/slitherin/detectors/price_manipulation.py index 59ebd87..b321bfd 100644 --- a/slitherin/detectors/price_manipulation.py +++ b/slitherin/detectors/price_manipulation.py @@ -1,11 +1,10 @@ from typing import List from slither.utils.output import Output from slither.detectors.abstract_detector import AbstractDetector, DetectorClassification -from slither.core.declarations import Function -from slither.slithir.operations.event_call import EventCall from slither.slithir.operations.high_level_call import HighLevelCall from slither.slithir.operations.internal_call import InternalCall -from slither.slithir.operations.binary import Binary, BinaryType +from slither.slithir.operations.solidity_call import SolidityCall +from slither.slithir.operations.binary import Binary from slither.analyses.data_dependency.data_dependency import is_dependent @@ -30,6 +29,9 @@ def _detect(self) -> List[Output]: for func in contract.functions: for n in func.nodes: for x in n.irs: + if isinstance(x, SolidityCall): + if x.function.name == "balance(address)" or x.function.name == "self.balance" or x.function.name == "this.balance()": + all_balance_vars.append((n, x._lvalue)) if isinstance(x, HighLevelCall): if str(x.function_name).lower() == "balanceof": all_balance_vars.append((n, x._lvalue)) diff --git a/tests/price_manipulation_test.sol b/tests/price_manipulation_test.sol index 4963514..85cc621 100644 --- a/tests/price_manipulation_test.sol +++ b/tests/price_manipulation_test.sol @@ -33,6 +33,10 @@ contract Test1 { price = getBalance() + mySupply() + 1; } + function test_vuln_7() external returns(uint256 price) { + price = address(token).balance / mySupply(); + } + function getBalance() public returns(uint256 bal) { bal = token.balanceOf(msg.sender); } From 1c6881fc967753a431bb4a5aaa1a339a0fa2d282 Mon Sep 17 00:00:00 2001 From: nikolay19 Date: Thu, 18 Apr 2024 09:37:21 +0300 Subject: [PATCH 46/48] update bench submodule in branch --- slitherin-benchmark | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/slitherin-benchmark b/slitherin-benchmark index 3500000..e7224b8 160000 --- a/slitherin-benchmark +++ b/slitherin-benchmark @@ -1 +1 @@ -Subproject commit 3500000097be15fa8bb5a52fa84c2ca8b11d8bbf +Subproject commit e7224b87fc769bee913e59dc1976dfd7de7bd209 From da6ca8c793e5b5212234ca4c3fa319134b910cf1 Mon Sep 17 00:00:00 2001 From: olegggatttor Date: Thu, 18 Apr 2024 13:35:43 +0300 Subject: [PATCH 47/48] add comma --- slitherin/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/slitherin/__init__.py b/slitherin/__init__.py index 26fd8a3..7298975 100644 --- a/slitherin/__init__.py +++ b/slitherin/__init__.py @@ -68,7 +68,7 @@ AAVEFlashloanCallbackDetector, PotentialArithmOverflow, CurveReadonlyReentrancy, - BalancerReadonlyReentrancy + BalancerReadonlyReentrancy, CurveVyperReentrancy, PriceManipulationDetector ] From 8659ab2eb80e22def226c65e59abf1d03b01e6e4 Mon Sep 17 00:00:00 2001 From: Nikita Kirillov Date: Tue, 7 May 2024 12:42:00 +0500 Subject: [PATCH 48/48] version update --- slitherin/consts.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/slitherin/consts.py b/slitherin/consts.py index 6820379..441c3d0 100644 --- a/slitherin/consts.py +++ b/slitherin/consts.py @@ -1,2 +1,2 @@ ARBITRUM_KEY = "SLITHERIN_ARBITRUM" -SLITHERIN_VERSION = "0.6.1" +SLITHERIN_VERSION = "0.7.0"