diff --git a/README.md b/README.md index ae4b59d..10acfcb 100644 --- a/README.md +++ b/README.md @@ -36,7 +36,7 @@ A whitehat that has discovered an exploit on a registered project will submit a > ⚠️ On Ethereum mainnet, it is critical that the whitehat uses a private mempool mechanism (e.g., Flashbots Protect with max privacy) to submit the transaction in order to prevent discovery of the exploit mechanism before the transaction is mined and the protocol can be paused! On other chains where sequencing cannot be practically frontrun, it may be sufficient to submit directly to the tx sequencer. -You can check out an example trace of a claim tx [here](https://phalcon.blocksec.com/explorer/tx/sepolia/0xd3ce2ef3a80a6461142020909acc8499e8b6e893073c77d534734d7d129abdc7). +You can check out an example trace of a claim tx [here](https://phalcon.blocksec.com/explorer/tx/sepolia/0x1b957becc3839f34d1e7f221eeb7353e6073b69da91ac717478a64c3fd2bf291). ## Writing Verifiers **Verifier**s must confirm that some critical invariants or health checks have been violated in the post-exploit state. Projects need to do the legwork of identifying a robust set of checks that would be considered critical enough to warrant pausing the entire protocol. These would typically be invariants that do not get checked during normal user interactions due to gas constraints. @@ -56,10 +56,10 @@ A **Verifier**'s `beforeExploit()` function returns arbitrary data. This will la If the **Verifier** applies any state changes (even transient ones), they should restrict the caller to the HoneyPause contract. Otherwise the **Verifier** may be maliciously invoked before the call to `claim()` to manipulate results. ## Writing Pausers -Because the exploit will be detailed onchain for all to see after the claim tx is made, **Pausers** should pause as much of the protocol to prevent replicating the exploit across related components (pools) of the system. Only the HoneyPause contract should be allowed to call `pause()` on the **Pauser** contract. The pause *must* occur when `Pauser.pause()` is called, and not in the payer, which is called immediately afterwards. +Because the exploit will be detailed onchain for all to see after the claim tx is made, **Pausers** should pause as much of the protocol to prevent replicating the exploit across related components (pools) of the system. Only the HoneyPause contract should be allowed to call `pause()` on the **Pauser** contract and the `bountyId` parameter should be confirmed to be one created by the project. The pause *must* occur when `Pauser.pause()` is called, and not in the payer, which is called immediately afterwards. ## Writing Payers -The **Payer** contract will be invoked by HoneyPause to transfer the bounty to the whitehat. Bounties can be in either ETH or ERC20. HoneyPause will surround the `payExploiter()` call with balance checks to ensure that payment has been delivered. The **Payer** contract should only allow the HoneyPause contract to call its `payExploiter()` function. +The **Payer** contract will be invoked by HoneyPause to transfer the bounty to the whitehat. Bounties can be in either ETH or ERC20. HoneyPause will surround the `payExploiter()` call with balance checks to ensure that payment has been delivered. The **Payer** contract should only allow the HoneyPause contract to call its `payExploiter()` function and the `bountyId` parameter should be confirmed to be one created by the project. The simplest **Payer** implementation will transfer the bounty directly out of a dedicated fund. Alternatively, a project may choose to keep the bounty value in its protocol and perform the conversion on-the-fly. In that case, the payment mechanism should be distinct from normal user operations on the protocol because the **Payer** will be invoked *after* the **Pauser**. Keep in mind that more complex payment mechanisms can open projects up to a secondary exploit. @@ -68,7 +68,7 @@ The simplest **Payer** implementation will transfer the bounty directly out of a | Chain | Address | |-------|---------| | Ethereum Mainnet | `TBD` | -| Ethereum Sepolia | [`0x5cd701310ae6e3185C29de433019C96efd298d60`](https://sepolia.etherscan.io/address/0x5cd701310ae6e3185c29de433019c96efd298d60) | +| Ethereum Sepolia | [`0x4730fe5DB07c5092893B87A3Cf6740cA25ffE52A`](https://sepolia.etherscan.io/address/0x4730fe5DB07c5092893B87A3Cf6740cA25ffE52A) | ## Credits diff --git a/script/demo/Dummies.sol b/script/demo/Dummies.sol index e26d995..fe5f14b 100644 --- a/script/demo/Dummies.sol +++ b/script/demo/Dummies.sol @@ -4,18 +4,6 @@ pragma solidity ^0.8.23; import { ERC20 } from 'solmate/tokens/ERC20.sol'; import { HoneyPause, IVerifier, IPauser, IPayer, IExploiter, ETH_TOKEN } from '../../src/HoneyPause.sol'; -contract TestPayer is IPayer { - constructor() payable {} - - function payExploiter(ERC20 token, address payable to, uint256 amount) external { - if (token == ETH_TOKEN) { - to.transfer(amount); - } else { - token.transfer(to, amount); - } - } -} - contract TestToken is ERC20 { constructor(string memory name, string memory symbol, uint8 decimals) ERC20(name, symbol, decimals) @@ -24,12 +12,4 @@ contract TestToken is ERC20 { function mint(address to, uint256 amount) external { _mint(to, amount); } -} - -contract SucceedingContract { - fallback() external payable {} -} - -contract FailingContract { - fallback() external payable { revert(); } -} +} \ No newline at end of file diff --git a/script/demo/SecretProtocol.sol b/script/demo/SecretProtocol.sol index 95ab25a..eed8b6b 100644 --- a/script/demo/SecretProtocol.sol +++ b/script/demo/SecretProtocol.sol @@ -2,6 +2,7 @@ pragma solidity ^0.8.23; import { ERC20 } from 'solmate/tokens/ERC20.sol'; +import { TestToken } from './Dummies.sol'; import { HoneyPause, IVerifier, IPauser, IPayer, IExploiter, ETH_TOKEN } from '../../src/HoneyPause.sol'; contract SecretProtocol { @@ -9,7 +10,7 @@ contract SecretProtocol { bytes3 public hash; bool public paused; - event PreimageFound(bytes32 preimage); + event PreimageFound(string preimage); event Paused(); constructor(bytes3 hash_, address pauser_) { @@ -17,9 +18,9 @@ contract SecretProtocol { pauser = pauser_; } - function solve(bytes32 preimage) external { + function solve(string memory preimage) external { require(!paused, 'paused'); - if (hash != 0 && bytes3(keccak256(abi.encode(preimage))) == hash) { + if (hash != 0 && bytes3(keccak256(bytes(preimage))) == hash) { emit PreimageFound(preimage); hash = 0; } @@ -51,8 +52,8 @@ contract SecretProtocolVerifier is IVerifier { contract SecretExploiter is IExploiter { function exploit(bytes memory exploiterData) external { - (SecretProtocol proto, bytes32 preimage) = - abi.decode(exploiterData, (SecretProtocol, bytes32)); + (SecretProtocol proto, string memory preimage) = + abi.decode(exploiterData, (SecretProtocol, string)); proto.solve(preimage); } } @@ -60,27 +61,152 @@ contract SecretExploiter is IExploiter { contract SecretProtocolPauser is IPauser { HoneyPause immutable honey; SecretProtocol public immutable proto; + uint256 immutable public bountyId; - constructor(HoneyPause honey_, SecretProtocol proto_) { + constructor( + SecretProtocol proto_, + HoneyPause honey_, + uint256 bountyId_ + ) { honey = honey_; proto = proto_; + bountyId = bountyId_; } - function pause() external { + function pause(uint256 bountyId_) external { require(msg.sender == address(honey), 'not honey'); + require(bountyId_ == bountyId, 'wrong bounty'); proto.pause(); } } contract SecretProtocolPayer is IPayer { HoneyPause immutable honey; + uint256 immutable bountyId; - constructor(HoneyPause honey_) { + constructor(HoneyPause honey_, uint256 bountyId_) { honey = honey_; + bountyId = bountyId_; } - function payExploiter(ERC20 token, address payable to, uint256 amount) external { + function payExploiter( + uint256 bountyId_, + ERC20 token, + address payable to, + uint256 amount + ) + external + { require(msg.sender == address(honey), 'not honey'); + require(bountyId_ == bountyId, 'wrong bounty'); token.transfer(to, amount); } +} + +contract SecretProtocolBountyDeployer { + event Deployed( + uint256 bountyId, + SecretProtocol proto, + SecretProtocolPauser pauser, + SecretProtocolVerifier verifier, + SecretProtocolPayer payer + ); + + SecretProtocol immutable public proto; + SecretProtocolPauser immutable public pauser; + SecretProtocolVerifier immutable public verifier; + SecretProtocolPayer immutable public payer; + uint256 immutable public bountyId; + + constructor( + HoneyPause honey, + string memory name, + TestToken token, + uint256 amount, + bytes3 hash, + address operator + ) { + bountyId = honey.bountyCount() + 1; + address pauserAddress = _getDeployedAddress(address(this), 2); + proto = new SecretProtocol(hash, pauserAddress); + pauser = new SecretProtocolPauser(proto, honey, bountyId); + assert(address(pauser) == pauserAddress); + verifier = new SecretProtocolVerifier(proto); + payer = new SecretProtocolPayer(honey, bountyId); + token.mint(address(payer), amount); + uint256 bountyId_ = honey.add({ + name: name, + payoutToken: token, + payoutAmount: amount, + verifier: verifier, + pauser: pauser, + payer: payer, + operator: operator + }); + assert(bountyId_ == bountyId); + emit Deployed(bountyId, proto, pauser, verifier, payer); + } + + function _getDeployedAddress(address deployer, uint32 deployNonce) + private + pure + returns (address deployed) + { + assembly { + mstore(0x02, shl(96, deployer)) + let rlpNonceLength + switch gt(deployNonce, 0xFFFFFF) + case 1 { // 4 byte nonce + rlpNonceLength := 5 + mstore8(0x00, 0xD8) + mstore8(0x16, 0x84) + mstore(0x17, shl(224, deployNonce)) + } + default { + switch gt(deployNonce, 0xFFFF) + case 1 { + // 3 byte nonce + rlpNonceLength := 4 + mstore8(0x16, 0x83) + mstore(0x17, shl(232, deployNonce)) + } + default { + switch gt(deployNonce, 0xFF) + case 1 { + // 2 byte nonce + rlpNonceLength := 3 + mstore8(0x16, 0x82) + mstore(0x17, shl(240, deployNonce)) + } + default { + switch gt(deployNonce, 0x7F) + case 1 { + // 1 byte nonce >= 0x80 + rlpNonceLength := 2 + mstore8(0x16, 0x81) + mstore8(0x17, deployNonce) + } + default { + rlpNonceLength := 1 + switch iszero(deployNonce) + case 1 { + // zero nonce + mstore8(0x16, 0x80) + } + default { + // 1 byte nonce < 0x80 + mstore8(0x16, deployNonce) + } + } + } + } + } + mstore8(0x00, add(0xD5, rlpNonceLength)) + mstore8(0x01, 0x94) + deployed := and( + keccak256(0x00, add(0x16, rlpNonceLength)), + 0xffffffffffffffffffffffffffffffffffffffff + ) + } + } } \ No newline at end of file diff --git a/script/demo/Testnet.s.sol b/script/demo/Testnet.s.sol index 7dd565b..a56ab07 100644 --- a/script/demo/Testnet.s.sol +++ b/script/demo/Testnet.s.sol @@ -6,12 +6,11 @@ import { ERC20 } from 'solmate/tokens/ERC20.sol'; import { HoneyPause, IVerifier, IPauser, IPayer, ETH_TOKEN } from '../../src/HoneyPause.sol'; import { SecretProtocol, - SecretProtocolVerifier, SecretExploiter, - SecretProtocolPauser, - SecretProtocolPayer + SecretProtocolBountyDeployer, + SecretProtocolVerifier } from './SecretProtocol.sol'; -import { TestPayer, TestToken, SucceedingContract, FailingContract } from './Dummies.sol'; +import { TestToken } from './Dummies.sol'; contract Testnet is Script { uint256 deployerKey; @@ -52,61 +51,31 @@ contract Testnet is Script { new TestToken('USDC', 'USDC', 6); } - function exploitProtocol(uint256 bountyId, SecretProtocol proto, bytes32 preimage) external { - require(bytes3(keccak256(abi.encode(preimage))) == proto.hash(), 'invalid preimage'); + function exploitProtocol(uint256 bountyId, string memory preimage) external { + (,,, IVerifier verifier,,) = honey.getBounty(bountyId); + SecretProtocol proto = SecretProtocolVerifier(address(verifier)).proto(); + require(bytes3(keccak256(bytes(preimage))) == proto.hash(), 'invalid preimage'); _broadcast(); SecretExploiter exploiter = new SecretExploiter(); _broadcast(); honey.claim(bountyId, payable(tx.origin), exploiter, abi.encode(proto, preimage), ""); } - function registerUsdcProtocol(string memory name, uint256 amount, bytes3 hash) external { - address pauserAddress = vm.computeCreateAddress(deployer, vm.getNonce(deployer) + 2); + function registerUsdcProtocol(string memory name, uint256 amount, string memory preimage) external { + bytes3 hash = bytes3(keccak256(bytes(preimage))); _broadcast(); - SecretProtocol proto = new SecretProtocol(hash, pauserAddress); - _broadcast(); - IVerifier verifier = new SecretProtocolVerifier(proto); - _broadcast(); - IPauser pauser = new SecretProtocolPauser(honey, proto); - assert(address(pauser) == pauserAddress); - _broadcast(); - IPayer payer = new SecretProtocolPayer(honey); - _broadcast(); - usdc.mint(address(payer), amount); - _broadcast(); - honey.add({ - name: name, - payoutToken: usdc, - payoutAmount: amount, - verifier: verifier, - pauser: pauser, - payer: payer, - operator: operator - }); + SecretProtocolBountyDeployer d = new SecretProtocolBountyDeployer( + honey, + name, + usdc, + amount, + hash, + operator + ); + require(honey.verifyBountyCanPay(d.bountyId(), payable(tx.origin)), 'cannot pay'); } function _broadcast() private { vm.broadcast(deployerKey); } - - function _deployPayerContract(ERC20 token, uint256 amount) private returns (IPayer) { - _broadcast(); - if (token == ETH_TOKEN) { - return new TestPayer{value: amount}(); - } - IPayer payer = new TestPayer(); - _broadcast(); - TestToken(address(token)).mint(address(payer), amount); - return payer; - } - - function _deploySucceedingContract() private returns (address) { - _broadcast(); - return address(new SucceedingContract()); - } - - function _deployFailingContract() private returns (address) { - _broadcast(); - return address(new FailingContract()); - } } \ No newline at end of file diff --git a/src/HoneyPause.sol b/src/HoneyPause.sol index 8fb7681..a48de3c 100644 --- a/src/HoneyPause.sol +++ b/src/HoneyPause.sol @@ -27,8 +27,10 @@ interface IVerifier { /// @dev Interface for a protocol's pauser contract. interface IPauser { /// @dev Pause a protocol. - /// The implementer MUST check that the caller of this function is the HoneyPause contract. - function pause() external; + /// The implementer MUST check that the caller of this function is the HoneyPause contract + /// and that the `bountyId` is one they recognize as their own. + /// @param bountyId ID of the bounty that triggered it. + function pause(uint256 bountyId) external; } /// @dev Interface for an exploit contract provided by a whitehat making a claim. @@ -41,13 +43,15 @@ interface IExploiter { /// @dev Interface for a protocol's payer contract. interface IPayer { /// @dev Pay the whitehat the bounty after proving an exploit was possible. - /// The implementer MUST check that the caller of this function is the HoneyPause contract. - /// This payment mechanism should be distinct from typical protocol + /// The implementer MUST check that the caller of this function is the HoneyPause contract + /// and that the `bountyId` is one they recognize as their own. + /// This payment mechanism should be distinct from user operations /// interactions because this function will be called AFTER the pauser has been invoked. + /// @param bountyId ID of the bounty that triggered it. /// @param token The bounty token. /// @param to The receving account for payment. /// @param amount The bounty amount. - function payExploiter(ERC20 token, address payable to, uint256 amount) external; + function payExploiter(uint256 bountyId, ERC20 token, address payable to, uint256 amount) external; } /// @dev ERC20 type alias for ETH. @@ -312,20 +316,26 @@ contract HoneyPause { } } // Pause the protocol. - address(bounty.pauser).safeCall(abi.encodeCall(bounty.pauser.pause, ())); + address(bounty.pauser).safeCall(abi.encodeCall(bounty.pauser.pause, (bountyId))); // Pay the bounty. - _payout(bounty.payer, bounty.payoutToken, payReceiver, bounty.payoutAmount); + _payout(bountyId, bounty.payer, bounty.payoutToken, payReceiver, bounty.payoutAmount); } // Call a bounty's payer contract and verify that it transferred the payment. - function _payout(IPayer payer, ERC20 token, address payable to, uint256 amount) + function _payout( + uint256 bountyId, + IPayer payer, + ERC20 token, + address payable to, + uint256 amount + ) internal { uint256 balBefore = _balanceOf(token, to); address(payer).safeCall(abi.encodeCall( payer.payExploiter, - (token, to, amount) + (bountyId, token, to, amount) )); uint256 balAfter = _balanceOf(token, to); if (balBefore > balAfter || balAfter - balBefore < amount) { diff --git a/test/HoneyPause.t.sol b/test/HoneyPause.t.sol index dbf6aab..5fbef03 100644 --- a/test/HoneyPause.t.sol +++ b/test/HoneyPause.t.sol @@ -54,38 +54,46 @@ contract HoneyPauseTest is Test { function test_payout_canPayERC20() external { IPayer payer = new TestERC20Payer(testToken, TEST_BOUNTY_AMOUNT); - honey.__payout(payer, testToken, RECEIVER, TEST_BOUNTY_AMOUNT); + vm.expectEmit(true, true, true, true); + emit TestERC20Payer.PayExploiterCalled(1337, testToken, RECEIVER, TEST_BOUNTY_AMOUNT); + honey.__payout(1337, payer, testToken, RECEIVER, TEST_BOUNTY_AMOUNT); assertEq(testToken.balanceOf(RECEIVER), TEST_BOUNTY_AMOUNT); } function test_payout_canPayZeroERC20() external { IPayer payer = new TestERC20Payer(testToken, 0); - honey.__payout(payer, testToken, RECEIVER, 0); + vm.expectEmit(true, true, true, true); + emit TestERC20Payer.PayExploiterCalled(1337, testToken, RECEIVER, 0); + honey.__payout(1337, payer, testToken, RECEIVER, 0); assertEq(testToken.balanceOf(RECEIVER), 0); } function test_payout_failsIfUnderpaysERC20() external { IPayer payer = new TestERC20Payer(testToken, TEST_BOUNTY_AMOUNT - 1); vm.expectRevert(InsufficientPayoutError.selector); - honey.__payout(payer, testToken, RECEIVER, TEST_BOUNTY_AMOUNT); + honey.__payout(1337, payer, testToken, RECEIVER, TEST_BOUNTY_AMOUNT); } function test_payout_canPayEth() external { IPayer payer = new TestEthPayer{value: TEST_BOUNTY_AMOUNT}(); - honey.__payout(payer, ETH_TOKEN, RECEIVER, TEST_BOUNTY_AMOUNT); + vm.expectEmit(true, true, true, true); + emit TestERC20Payer.PayExploiterCalled(1337, ETH_TOKEN, RECEIVER, TEST_BOUNTY_AMOUNT); + honey.__payout(1337, payer, ETH_TOKEN, RECEIVER, TEST_BOUNTY_AMOUNT); assertEq(RECEIVER.balance, TEST_BOUNTY_AMOUNT); } function test_payout_canPayZeroEth() external { IPayer payer = new TestEthPayer(); - honey.__payout(payer, ETH_TOKEN, RECEIVER, 0); + vm.expectEmit(true, true, true, true); + emit TestERC20Payer.PayExploiterCalled(1337, ETH_TOKEN, RECEIVER, 0); + honey.__payout(1337, payer, ETH_TOKEN, RECEIVER, 0); assertEq(RECEIVER.balance, 0); } function test_payout_failsIfUnderpaysEth() external { IPayer payer = new TestEthPayer{value: TEST_BOUNTY_AMOUNT - 1}(); vm.expectRevert(InsufficientPayoutError.selector); - honey.__payout(payer, ETH_TOKEN, RECEIVER, TEST_BOUNTY_AMOUNT); + honey.__payout(1337, payer, ETH_TOKEN, RECEIVER, TEST_BOUNTY_AMOUNT); } function test_sandboxExploit_revertsOnSuccess() external { @@ -361,7 +369,7 @@ contract HoneyPauseTest is Test { uint256 bountyId = _addTestBounty(); IExploiter exploiter = new TestExploiter(); vm.expectEmit(true, true, true, true); - emit TestPauser.PauseCalled(); + emit TestPauser.PauseCalled(bountyId); honey.claim(bountyId, RECEIVER, exploiter, "", ""); } @@ -545,10 +553,10 @@ contract TestExploiter is IExploiter { } contract TestPauser is IPauser { - event PauseCalled(); + event PauseCalled(uint256 bountyId); - function pause() external { - emit PauseCalled(); + function pause(uint256 bountyId) external { + emit PauseCalled(bountyId); } } @@ -575,11 +583,14 @@ contract TestVerifier is IVerifier { } contract TestERC20Payer is IPayer { + event PayExploiterCalled(uint256 bountyId, ERC20 token, address payable to, uint256 amount); + constructor(TestERC20 token, uint256 reserve) { token.mint(address(this), reserve) ; } - function payExploiter(ERC20 token, address payable to, uint256) external { + function payExploiter(uint256 bountyId, ERC20 token, address payable to, uint256 amount) external { + emit PayExploiterCalled(bountyId, token, to, amount); token.transfer(to, token.balanceOf(address(this))); } } @@ -613,19 +624,27 @@ contract FailingContract { } contract TestEthPayer is IPayer { + event PayExploiterCalled(uint256 bountyId, ERC20 token, address payable to, uint256 amount); constructor() payable {} - function payExploiter(ERC20 token, address payable to, uint256) external { + function payExploiter(uint256 bountyId, ERC20 token, address payable to, uint256 amount) external { + emit PayExploiterCalled(bountyId, token, to, amount); assert(token == ETH_TOKEN); return to.transfer(address(this).balance); } } contract TestHoneyPause is HoneyPause { - function __payout(IPayer payer, ERC20 token, address payable to, uint256 amount) + function __payout( + uint256 bountyId, + IPayer payer, + ERC20 token, + address payable to, + uint256 amount + ) external { - _payout(payer, token, to, amount); + _payout(bountyId, payer, token, to, amount); } function __testSetBountyVerifier(uint256 bountyId, IVerifier verifier)