diff --git a/.github/workflows/benchmark.yml b/.github/workflows/benchmark.yml index 66c4349..013bd3a 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,12 +45,26 @@ 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 -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 -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: @@ -89,6 +104,15 @@ 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 -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: @@ -99,4 +123,3 @@ jobs: with: name: oz_extra path: slitherin-benchmark/oz_extra.csv - diff --git a/.github/workflows/old_version.yml b/.github/workflows/old_version.yml new file mode 100644 index 0000000..758dbc8 --- /dev/null +++ b/.github/workflows/old_version.yml @@ -0,0 +1,118 @@ +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 + diff --git a/README.md b/README.md index 983d9fa..6bdc9ad 100644 --- a/README.md +++ b/README.md @@ -1,4 +1,4 @@ -# Slitherin by Pessimistic.io +# Slitherin by Pessimistic.io [![Blog](https://img.shields.io/badge/Blog-Link-blue?style=flat-square&logo=appveyor?logo=data:https://pessimistic.io/favicon.ico)](https://blog.pessimistic.io/) [![Our Website](https://img.shields.io/badge/By-pessimistic.io-green?style=flat-square&logo=appveyor?logo=data:https://pessimistic.io/favicon.ico)](https://pessimistic.io/) @@ -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:** 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 diff --git a/docs/balancer/readonly_reentrancy.md b/docs/balancer/readonly_reentrancy.md new file mode 100644 index 0000000..7094fb8 --- /dev/null +++ b/docs/balancer/readonly_reentrancy.md @@ -0,0 +1,24 @@ +# Balancer Readonly Reentrancy + +## Configuration + +- Check: `pess-balancer-readonly-reentrancy` +- Severity: `High` +- Confidence: `Medium` + +## Description + +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) + +## Recommendation + +- [Official Balancer recomendation](https://docs.balancer.fi/concepts/advanced/valuing-bpt/valuing-bpt.html#on-chain-price-evaluation) 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/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/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/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/package-lock.json b/package-lock.json index cc36f30..8fb9792 100644 --- a/package-lock.json +++ b/package-lock.json @@ -5,9 +5,33 @@ "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/setup.py b/setup.py index 66bfb56..dd011a6 100644 --- a/setup.py +++ b/setup.py @@ -30,6 +30,7 @@ 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-benchmark b/slitherin-benchmark index 12792f9..e7224b8 160000 --- a/slitherin-benchmark +++ b/slitherin-benchmark @@ -1 +1 @@ -Subproject commit 12792f91792dfe5a90ed9a0fc955606082b92305 +Subproject commit e7224b87fc769bee913e59dc1976dfd7de7bd209 diff --git a/slitherin/__init__.py b/slitherin/__init__.py index cd9e78b..7298975 100644 --- a/slitherin/__init__.py +++ b/slitherin/__init__.py @@ -29,11 +29,17 @@ 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 +from slitherin.detectors.curve.curve_readonly_reentrancy import CurveReadonlyReentrancy +from slitherin.detectors.balancer.balancer_readonly_reentrancy import BalancerReadonlyReentrancy +from slitherin.detectors.vyper.reentrancy_curve_vyper_version import CurveVyperReentrancy +from slitherin.detectors.price_manipulation import PriceManipulationDetector artbitrum_detectors = [ ArbitrumPrevrandaoDifficulty, ArbitrumBlockNumberTimestamp, + ArbitrumChainlinkPriceFeed ] plugin_detectors = artbitrum_detectors + [ @@ -60,7 +66,11 @@ Ecrecover, PublicVsExternal, AAVEFlashloanCallbackDetector, - PotentialArithmOverflow + PotentialArithmOverflow, + CurveReadonlyReentrancy, + BalancerReadonlyReentrancy, + CurveVyperReentrancy, + PriceManipulationDetector ] plugin_printers = [] 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" 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 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/balancer/balancer_readonly_reentrancy.py b/slitherin/detectors/balancer/balancer_readonly_reentrancy.py new file mode 100644 index 0000000..4dba564 --- /dev/null +++ b/slitherin/detectors/balancer/balancer_readonly_reentrancy.py @@ -0,0 +1,114 @@ +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 = "Balancer readonly-reentrancy" + IMPACT = DetectorClassification.HIGH + CONFIDENCE = DetectorClassification.MEDIUM + + 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 = "Check docs" + + 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 not n.name: + continue + 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 = 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_balancer_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 = [ + "Balancer 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/slitherin/detectors/curve/curve_readonly_reentrancy.py b/slitherin/detectors/curve/curve_readonly_reentrancy.py new file mode 100644 index 0000000..c3d641a --- /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/curve_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/slitherin/detectors/nft_approve_warning.py b/slitherin/detectors/nft_approve_warning.py index 8f17299..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,12 +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)"] + _signatures=["transferFrom(address,address,uint256)", "safeTransferFrom(address,address,uint256,bytes)", "safeTransferFrom(address,address,uint256)", + 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 = [ @@ -48,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/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 diff --git a/slitherin/detectors/price_manipulation.py b/slitherin/detectors/price_manipulation.py new file mode 100644 index 0000000..b321bfd --- /dev/null +++ b/slitherin/detectors/price_manipulation.py @@ -0,0 +1,71 @@ +from typing import List +from slither.utils.output import Output +from slither.detectors.abstract_detector import AbstractDetector, DetectorClassification +from slither.slithir.operations.high_level_call import HighLevelCall +from slither.slithir.operations.internal_call import InternalCall +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 + + +class PriceManipulationDetector(AbstractDetector): + ARGUMENT = 'pess-price-manipulation' # slither will launch the detector with slither.py --detect mydetector + 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/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 = 'Avoid possible manipulations of calculations because of external transfers.' + + def _detect(self) -> List[Output]: + all_balance_vars = [] + all_supply_vars = [] + all_binary_ops = [] + for contract in self.contracts: + if not contract.is_interface: + 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)) + 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/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, diff --git a/slitherin/detectors/strange_setter.py b/slitherin/detectors/strange_setter.py index 323dc9b..62c7a02 100644 --- a/slitherin/detectors/strange_setter.py +++ b/slitherin/detectors/strange_setter.py @@ -3,6 +3,8 @@ 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 class StrangeSetter(AbstractDetector): @@ -51,9 +53,18 @@ 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) + 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) 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..34c3ae5 --- /dev/null +++ b/slitherin/detectors/vyper/reentrancy_curve_vyper_version.py @@ -0,0 +1,40 @@ +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): + 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/slitherin/napalm.py b/slitherin/napalm.py new file mode 100644 index 0000000..5e60d04 --- /dev/null +++ b/slitherin/napalm.py @@ -0,0 +1,18 @@ +import napalm.package +import slitherin as slitherin + + +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. + """ + _include = ("detectors",) + + return [ + collection + for collection in napalm.package.entry_point(slitherin) + if collection.collection_name in _include + ] 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/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/balancer/readonly_reentrancy_test.sol b/tests/balancer/readonly_reentrancy_test.sol new file mode 100644 index 0000000..77499fb --- /dev/null +++ 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); + } +} 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 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 diff --git a/tests/nft_approve_warning_test.sol b/tests/nft_approve_warning_test.sol index a53f7b0..735e0b9 100644 --- a/tests/nft_approve_warning_test.sol +++ b/tests/nft_approve_warning_test.sol @@ -36,3 +36,56 @@ 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), ""); + } +} + + +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); + } +} diff --git a/tests/price_manipulation_test.sol b/tests/price_manipulation_test.sol new file mode 100644 index 0000000..85cc621 --- /dev/null +++ b/tests/price_manipulation_test.sol @@ -0,0 +1,47 @@ +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 test_vuln_7() external returns(uint256 price) { + price = address(token).balance / mySupply(); + } + + function getBalance() public returns(uint256 bal) { + bal = token.balanceOf(msg.sender); + } + + function mySupply() public returns (uint256) { + return 100; + } +} \ No newline at end of file diff --git a/tests/strange_setter_test.sol b/tests/strange_setter_test.sol index d2475c3..58108ed 100644 --- a/tests/strange_setter_test.sol +++ b/tests/strange_setter_test.sol @@ -87,3 +87,13 @@ contract OkConstructor { init = true; } } + +contract TestInheritance is StrangeSetter{ + constructor(uint256 _toSet) StrangeSetter(_toSet) {} +} + +contract TestNewContract { + constructor(uint256 _toSet) { + new TestInheritance(_toSet); + } +} \ No newline at end of file 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