From c894abe0dc7f8fe829e9d58958f43615d7d455e2 Mon Sep 17 00:00:00 2001 From: Nikita Kirillov Date: Wed, 18 Oct 2023 11:00:40 +0500 Subject: [PATCH 01/13] 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/13] 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/13] 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/13] 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/13] 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 c1b067d4443c774d4afad4921a6abb0b3a9cd324 Mon Sep 17 00:00:00 2001 From: Nikita Kirillov Date: Fri, 22 Mar 2024 14:16:49 +0500 Subject: [PATCH 06/13] 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 07/13] 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 08/13] 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 09/13] 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 10/13] 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 11/13] 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 1c6881fc967753a431bb4a5aaa1a339a0fa2d282 Mon Sep 17 00:00:00 2001 From: nikolay19 Date: Thu, 18 Apr 2024 09:37:21 +0300 Subject: [PATCH 12/13] 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 13/13] 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 ]