diff --git a/README.md b/README.md index ae4b59d..7214d7a 100644 --- a/README.md +++ b/README.md @@ -13,7 +13,7 @@ Projects register bounties with the HoneyPause contract via the `add()` function 1. A bounty token and amount, but no deposit*. 2. The address of a custom [`Verifier`](ause/blob/main/src/HoneyPause.sol#L8) contract which asserts onchain state invariants that would be violated in the case of an exploit. Examples could be an AMM's reserve violating the constant product formula or a lending protocol incurring bad debt. 3. The address of a custom [`Pauser`](./src/HoneyPause.sol#L28) contract which is authorized to pause/freeze the protocol when called by the HoneyPause contract. -4. The address of a custom [`Payer`](./src/HoneyPause.sol#L42) contract which must pay the bounty to the whitehat when called by the HoneyPause contract. +4. The address of a custom [`Payer`](./src/HoneyPause.sol#L44) contract which must pay the bounty to the whitehat when called by the HoneyPause contract. 5. An operator account for the bounty, who will be able to modify the bounty. > \* Note that the HoneyPause contract never custodies bounties. It is up to the project's **Payer** contract to surface funds to cover the bounty when called. @@ -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) (older version). ## 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 | [`0x81cb0ba5E7B724B03C8B64A5FAF4f2C655aD15E4`](https://sepolia.etherscan.io/address/0x81cb0ba5E7B724B03C8B64A5FAF4f2C655aD15E4) | ## Credits diff --git a/assets/flow.jpg b/assets/flow.jpg index c1a51c6..2bf8aec 100644 Binary files a/assets/flow.jpg and b/assets/flow.jpg differ 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..1532466 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,32 @@ 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 + ); + (bool canPay,) = honey.verifyBountyCanPay(d.bountyId(), payable(tx.origin)); + require(canPay, '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..9ca3e24 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. @@ -200,9 +204,10 @@ contract HoneyPause { bytes memory exploiterData, bytes memory verifierData ) - external + external returns (uint256 payAmount) { - Bounty memory bounty = _claim({ + Bounty memory bounty; + (bounty, payAmount) = _claim({ bountyId: bountyId, payReceiver: payReceiver, exploiter: exploiter, @@ -210,7 +215,7 @@ contract HoneyPause { verifierData: verifierData, skipExploit: false }); - emit Claimed(bountyId, bounty.payoutToken, bounty.payoutAmount); + emit Claimed(bountyId, bounty.payoutToken, payAmount); } /// @notice Carries out an exploit, verifies it, then reverts the call frame. @@ -239,7 +244,7 @@ contract HoneyPause { verifier.assertExploit, (verifierData, verifierStateData) )); - revert SandboxSucceededError(); + revert SandboxSucceededError(''); } /// @notice Check whether a bounty can actually pay out its reward. @@ -247,15 +252,24 @@ contract HoneyPause { /// determination. All state is reverted before returning so this can /// be safely called on-chain but most likely it will be consumed off-chain /// via eth_call. + /// @param bountyId ID of a valid, active bounty. + /// @param payReceiver Recepient of bounty. + /// @return bountyCanPay Whether the payer sent at least the bounty amount to the receiver. + /// @return payAmount Actual amount sent to receiver (may be more than bounty amount). function verifyBountyCanPay(uint256 bountyId, address payable payReceiver) - external returns (bool bountyCanPay) + external returns (bool bountyCanPay, uint256 payAmount) { try this.sandboxTryPayBounty(bountyId, payReceiver) { // Should always fail. assert(false); } catch (bytes memory errData) { - return LibSandbox.handleSandboxCallRevert(errData, true); + if (!LibSandbox.handleSandboxCallRevert(errData, true)) { + return (false, 0); + } + // The data inside the SandboxSucceededError is the pay amount. + payAmount = abi.decode(abi.decode(errData.skip(4), (bytes)), (uint256)); } + return (true, payAmount); } /// @notice Mimics the logic for a successful claim() to verify that a bounty can @@ -265,7 +279,7 @@ contract HoneyPause { function sandboxTryPayBounty(uint256 bountyId, address payable payReceiver) external { - _claim({ + (, uint256 payAmount) = _claim({ bountyId: bountyId, payReceiver: payReceiver, exploiter: IExploiter(address(0)), @@ -275,7 +289,7 @@ contract HoneyPause { // it does minus the exploit verification. skipExploit: true }); - revert SandboxSucceededError(); + revert SandboxSucceededError(abi.encode(payAmount)); } /// @dev The logic of claim(), refactored out for sandboxTryPayBounty(). @@ -288,7 +302,7 @@ contract HoneyPause { bool skipExploit ) internal - returns (Bounty memory bounty) + returns (Bounty memory bounty, uint256 payAmount) { bounty = getBounty[bountyId]; if (bounty.operator == address(0)) { @@ -312,25 +326,40 @@ 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); + payAmount = _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) + // Call a bounty's payer contract, verify that it transferred the payment, and + // return the amount transferred. + function _payout( + uint256 bountyId, + IPayer payer, + ERC20 token, + address payable to, + uint256 amount + ) internal + returns (uint256 payAmount) { 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) { revert InsufficientPayoutError(); } + return balAfter - balBefore; } // Get the balance of a token (or ETH) of an account. diff --git a/src/LibSandbox.sol b/src/LibSandbox.sol index d8f28a6..12367ad 100644 --- a/src/LibSandbox.sol +++ b/src/LibSandbox.sol @@ -4,7 +4,7 @@ pragma solidity ^0.8.24; import { LibBytes } from './LibBytes.sol'; error SandboxFailedError(bytes innerError); -error SandboxSucceededError(); +error SandboxSucceededError(bytes data); library LibSandbox { using LibBytes for bytes; diff --git a/test/HoneyPause.t.sol b/test/HoneyPause.t.sol index dbf6aab..2877d2b 100644 --- a/test/HoneyPause.t.sol +++ b/test/HoneyPause.t.sol @@ -38,9 +38,9 @@ contract HoneyPauseTest is Test { TestHoneyPause honey = new TestHoneyPause(); TestERC20 testToken = new TestERC20(); bytes TEST_ERROR = abi.encodeWithSelector(TestError.selector, 'FAILED'); - bytes SANDBOX_SUCCEEDED_ERROR = abi.encodeWithSelector(SandboxSucceededError.selector); + bytes EMPTY_SANDBOX_SUCCEEDED_ERROR = abi.encodeWithSelector(SandboxSucceededError.selector, ""); bytes WRAPPED_SANDBOX_SUCCEEDED_ERROR = abi.encodeWithSelector(SandboxFailedError.selector, - abi.encodeWithSelector(SandboxSucceededError.selector) + abi.encodeWithSelector(SandboxSucceededError.selector, "") ); function test_errorSelectorsCanBeSandboxed() external { @@ -54,44 +54,52 @@ 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 { IVerifier verifier = new TestVerifier(); IExploiter exploiter = new TestExploiter(); - vm.expectRevert(SandboxSucceededError.selector); + vm.expectRevert(EMPTY_SANDBOX_SUCCEEDED_ERROR); honey.sandboxExploit(exploiter, verifier, "", ""); } @@ -128,7 +136,7 @@ contract HoneyPauseTest is Test { function test_sandboxExploit_wrapsSandboxSuccessFromVerifierBeforeExploitCall() external { IVerifier verifier = IVerifier(_createFailingFnContract( IVerifier.beforeExploit.selector, - SANDBOX_SUCCEEDED_ERROR + EMPTY_SANDBOX_SUCCEEDED_ERROR )); IExploiter exploiter = new TestExploiter(); vm.expectRevert(WRAPPED_SANDBOX_SUCCEEDED_ERROR); @@ -138,7 +146,7 @@ contract HoneyPauseTest is Test { function test_sandboxExploit_wrapsSandboxSuccessFromVerifierAssertExploitCall() external { IVerifier verifier = IVerifier(_createFailingFnContract( IVerifier.assertExploit.selector, - SANDBOX_SUCCEEDED_ERROR + EMPTY_SANDBOX_SUCCEEDED_ERROR )); TestExploiter exploiter = new TestExploiter(); vm.expectRevert(WRAPPED_SANDBOX_SUCCEEDED_ERROR); @@ -149,7 +157,7 @@ contract HoneyPauseTest is Test { IVerifier verifier = new TestVerifier(); IExploiter exploiter = IExploiter(_createFailingFnContract( IExploiter.exploit.selector, - SANDBOX_SUCCEEDED_ERROR + EMPTY_SANDBOX_SUCCEEDED_ERROR )); vm.expectRevert(WRAPPED_SANDBOX_SUCCEEDED_ERROR); honey.sandboxExploit(exploiter, verifier, "", ""); @@ -168,7 +176,7 @@ contract HoneyPauseTest is Test { emit TestExploiter.ExploitCalled(exploiterData); vm.expectEmit(true, true, true, true); emit TestVerifier.AssertExploitCalled(verifierData, verifierStateData); - vm.expectRevert(SandboxSucceededError.selector); + vm.expectRevert(EMPTY_SANDBOX_SUCCEEDED_ERROR); honey.sandboxExploit(exploiter, verifier, exploiterData, verifierData); } @@ -341,8 +349,9 @@ contract HoneyPauseTest is Test { IExploiter exploiter = new TestExploiter(); vm.expectEmit(true, true, true, true); emit Claimed(bountyId, ETH_TOKEN, 100); - honey.claim(bountyId, RECEIVER, exploiter, "", ""); + uint256 payAmount = honey.claim(bountyId, RECEIVER, exploiter, "", ""); assertEq(honey.isBountyClaimed(bountyId), true); + assertEq(payAmount, TEST_BOUNTY_AMOUNT); } function test_claim_passesData() external { @@ -361,7 +370,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, "", ""); } @@ -423,15 +432,26 @@ contract HoneyPauseTest is Test { uint256 bountyId = _addTestBounty(); IExploiter exploiter = IExploiter(_createFailingFnContract( IExploiter.exploit.selector, - SANDBOX_SUCCEEDED_ERROR + EMPTY_SANDBOX_SUCCEEDED_ERROR )); - vm.expectRevert(SANDBOX_SUCCEEDED_ERROR); + vm.expectRevert(EMPTY_SANDBOX_SUCCEEDED_ERROR); honey.claim(bountyId, RECEIVER, exploiter, "", ""); } function test_verifyBountyCanPay_returnsTrueIfBountyPays() external { uint256 bountyId = _addTestBounty(); - assertEq(honey.verifyBountyCanPay(bountyId, RECEIVER), true); + (bool canPay, uint256 payAmount) = honey.verifyBountyCanPay(bountyId, RECEIVER); + assertEq(canPay, true); + assertEq(payAmount, TEST_BOUNTY_AMOUNT); + } + + function test_verifyBountyCanPay_returnsTrueIfBountyPaysTooMuch() external { + uint256 bountyId = _addTestBounty(); + (,,,,, IPayer payer) = honey.getBounty(bountyId); + payable(address(payer)).transfer(1); + (bool canPay, uint256 payAmount) = honey.verifyBountyCanPay(bountyId, RECEIVER); + assertEq(canPay, true); + assertEq(payAmount, TEST_BOUNTY_AMOUNT + 1); } function test_verifyBountyCanPay_doesNotModifyState() external { @@ -449,7 +469,9 @@ contract HoneyPauseTest is Test { IPayer.payExploiter.selector, TEST_ERROR ))); - assertEq(honey.verifyBountyCanPay(bountyId, RECEIVER), false); + (bool canPay, uint256 payAmount) = honey.verifyBountyCanPay(bountyId, RECEIVER); + assertEq(canPay, false); + assertEq(payAmount, 0); } function test_verifyBountyCanPay_returnsFalseIfPauserReverts() external { @@ -458,37 +480,92 @@ contract HoneyPauseTest is Test { IPauser.pause.selector, TEST_ERROR ))); - assertEq(honey.verifyBountyCanPay(bountyId, RECEIVER), false); + (bool canPay, uint256 payAmount) = honey.verifyBountyCanPay(bountyId, RECEIVER); + assertEq(canPay, false); + assertEq(payAmount, 0); } function test_verifyBountyCanPay_returnsFalseIfPayerDoesNotPayEnoughEth() external { uint256 bountyId = _addTestBounty(); honey.__testSetBountyPayer(bountyId, new TestEthPayer{value: TEST_BOUNTY_AMOUNT - 1}()); - assertEq(honey.verifyBountyCanPay(bountyId, RECEIVER), false); + (bool canPay, uint256 payAmount) = honey.verifyBountyCanPay(bountyId, RECEIVER); + assertEq(canPay, false); + assertEq(payAmount, 0); } function test_verifyBountyCanPay_returnsFalseIfPayerDoesNotPayEnoughErc20() external { uint256 bountyId = _addTestBounty(); honey.__testSetBountyPayer(bountyId, new TestERC20Payer(testToken, TEST_BOUNTY_AMOUNT - 1)); - assertEq(honey.verifyBountyCanPay(bountyId, RECEIVER), false); + (bool canPay, uint256 payAmount) = honey.verifyBountyCanPay(bountyId, RECEIVER); + assertEq(canPay, false); + assertEq(payAmount, 0); } function test_verifyBountyCanPay_returnsFalseIfPayerRevertsWithSandoxSucceeded() external { uint256 bountyId = _addTestBounty(); honey.__testSetBountyPayer(bountyId, IPayer(_createFailingFnContract( IPayer.payExploiter.selector, - SANDBOX_SUCCEEDED_ERROR + EMPTY_SANDBOX_SUCCEEDED_ERROR ))); - assertEq(honey.verifyBountyCanPay(bountyId, RECEIVER), false); + (bool canPay, uint256 payAmount) = honey.verifyBountyCanPay(bountyId, RECEIVER); + assertEq(canPay, false); + assertEq(payAmount, 0); } function test_verifyBountyCanPay_returnsFalseIfPauserRevertsWithSandoxSucceeded() external { uint256 bountyId = _addTestBounty(); honey.__testSetBountyPauser(bountyId, IPauser(_createFailingFnContract( IPauser.pause.selector, - SANDBOX_SUCCEEDED_ERROR + EMPTY_SANDBOX_SUCCEEDED_ERROR ))); - assertEq(honey.verifyBountyCanPay(bountyId, RECEIVER), false); + (bool canPay, uint256 payAmount) = honey.verifyBountyCanPay(bountyId, RECEIVER); + assertEq(canPay, false); + assertEq(payAmount, 0); + } + + function test_bountyCannotUseAnotherBountyPauserPayer() external { + MockPauser mockPauser = new MockPauser(); + MockPayer mockPayer = new MockPayer(); + + uint256 bountyAmount = 1 ether; + + // Create legitimate and a fake bounty with the mock contracts. + uint256 legitimateBountyId = honey.add( + "Legitimate", + testToken, + bountyAmount, + new TestVerifier(), + mockPauser, + mockPayer, + address(this) + ); + + uint256 fakeBountyId = honey.add( + "ExploitTest", + testToken, + bountyAmount, + new TestVerifier(), + mockPauser, + mockPayer, + address(this) + ); + + IExploiter exploiter = new TestExploiter(); + + // Set the valid bountyId in mock contracts to the legitimate bountyId. + mockPauser.setValidBountyId(legitimateBountyId); + mockPayer.setValidBountyId(legitimateBountyId); + + // Test that the pauser receives the correct bountyId and reverts against the fake bountyId. + vm.expectRevert("Unauthorized bountyId in pauser"); + honey.claim(fakeBountyId, payable(address(this)), exploiter, "", ""); + + // Temporarily allow pausing for the fake bounty to test payer logic. + mockPauser.setValidBountyId(fakeBountyId); + + // Test that the payer also correctly identifies and reverts against fake bountyId claims. + vm.expectRevert("Unauthorized bountyId in payer"); + honey.claim(fakeBountyId, payable(address(this)), exploiter, "", ""); } function _addTestBounty() @@ -536,6 +613,31 @@ contract HoneyPauseTest is Test { } } +contract MockPauser is IPauser { + uint256 private validBountyId; + + function setValidBountyId(uint256 validBountyId_) external { + validBountyId = validBountyId_; + } + + function pause(uint256 bountyId) external view { + require(bountyId == validBountyId, "Unauthorized bountyId in pauser"); + } +} + +contract MockPayer is IPayer { + uint256 private validBountyId; + + function setValidBountyId(uint256 validBountyId_) external { + validBountyId = validBountyId_; + } + + function payExploiter(uint256 bountyId, ERC20 token, address payable to, uint256 amount) external override { + require(bountyId == validBountyId, "Unauthorized bountyId in payer"); + token.transfer(to, amount); + } +} + contract TestExploiter is IExploiter { event ExploitCalled(bytes exploiterData); @@ -545,10 +647,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 +677,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 +718,29 @@ 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); } + + receive() external payable {} } 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) @@ -654,4 +769,4 @@ contract TestHoneyPause is HoneyPause { ) external pure { _validateBountyConfig({ operator: operator, pauser: pauser, verifier: verifier, payer: payer }); } -} \ No newline at end of file +}