diff --git a/README.md b/README.md index 24b2d25..7214d7a 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/0x1b957becc3839f34d1e7f221eeb7353e6073b69da91ac717478a64c3fd2bf291). +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. @@ -68,7 +68,7 @@ The simplest **Payer** implementation will transfer the bounty directly out of a | Chain | Address | |-------|---------| | Ethereum Mainnet | `TBD` | -| Ethereum Sepolia | [`0x4730fe5DB07c5092893B87A3Cf6740cA25ffE52A`](https://sepolia.etherscan.io/address/0x4730fe5DB07c5092893B87A3Cf6740cA25ffE52A) | +| 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/Testnet.s.sol b/script/demo/Testnet.s.sol index a56ab07..1532466 100644 --- a/script/demo/Testnet.s.sol +++ b/script/demo/Testnet.s.sol @@ -72,7 +72,8 @@ contract Testnet is Script { hash, operator ); - require(honey.verifyBountyCanPay(d.bountyId(), payable(tx.origin)), 'cannot pay'); + (bool canPay,) = honey.verifyBountyCanPay(d.bountyId(), payable(tx.origin)); + require(canPay, 'cannot pay'); } function _broadcast() private { diff --git a/src/HoneyPause.sol b/src/HoneyPause.sol index a48de3c..9ca3e24 100644 --- a/src/HoneyPause.sol +++ b/src/HoneyPause.sol @@ -204,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, @@ -214,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. @@ -243,7 +244,7 @@ contract HoneyPause { verifier.assertExploit, (verifierData, verifierStateData) )); - revert SandboxSucceededError(); + revert SandboxSucceededError(''); } /// @notice Check whether a bounty can actually pay out its reward. @@ -251,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 @@ -269,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)), @@ -279,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(). @@ -292,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)) { @@ -318,11 +328,18 @@ contract HoneyPause { // Pause the protocol. address(bounty.pauser).safeCall(abi.encodeCall(bounty.pauser.pause, (bountyId))); // Pay the bounty. - _payout(bountyId, 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. + // Call a bounty's payer contract, verify that it transferred the payment, and + // return the amount transferred. function _payout( uint256 bountyId, IPayer payer, @@ -331,6 +348,7 @@ contract HoneyPause { uint256 amount ) internal + returns (uint256 payAmount) { uint256 balBefore = _balanceOf(token, to); address(payer).safeCall(abi.encodeCall( @@ -341,6 +359,7 @@ contract HoneyPause { 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 5fbef03..9e0bf80 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 { @@ -99,7 +99,7 @@ contract HoneyPauseTest is Test { 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, "", ""); } @@ -136,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); @@ -146,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); @@ -157,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, "", ""); @@ -176,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); } @@ -349,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 { @@ -431,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 { @@ -457,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 { @@ -466,37 +480,47 @@ 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 _addTestBounty() @@ -632,6 +656,8 @@ contract TestEthPayer is IPayer { assert(token == ETH_TOKEN); return to.transfer(address(this).balance); } + + receive() external payable {} } contract TestHoneyPause is HoneyPause {