fix: CREATE2 self-destruct bypass in onchain/src/LiquidityManager.sol (#921)
The previous guard blocked setFeeDestination when feeDestination.code.length > 0 but did not persist feeDestinationLocked — a revert undoes all state changes. An attacker could CREATE2-deploy bytecode to the EOA fee destination, triggering the block, then SELFDESTRUCT to clear the code, then call setFeeDestination again successfully (lock was never committed). Fix: detect bytecode at the current feeDestination first; if found, set feeDestinationLocked = true and RETURN (not revert) so the storage write is committed. A subsequent SELFDESTRUCT cannot undo a committed storage slot. Updated NatSpec documents both the protection and the remaining limitation (atomic CREATE2+SELFDESTRUCT in a single tx cannot be detected). Added testSetFeeDestination_CREATE2BytecodeDetection_Locks covering: set EOA → vm.etch (simulate CREATE2 deploy) → verify lock committed → vm.etch empty (simulate selfdestruct) → verify setter still blocked. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
This commit is contained in:
parent
18c19c66a6
commit
534382f785
2 changed files with 58 additions and 12 deletions
|
|
@ -1023,6 +1023,39 @@ contract LiquidityManagerTest is UniSwapHelper {
|
|||
freshLm.setFeeDestination(makeAddr("anyAddr"));
|
||||
}
|
||||
|
||||
/**
|
||||
* @notice CREATE2 bypass defense: if the current feeDestination acquires bytecode after being
|
||||
* set as an EOA, the next setFeeDestination call commits the lock WITHOUT reverting so
|
||||
* the lock persists even after a subsequent SELFDESTRUCT clears the bytecode.
|
||||
*/
|
||||
function testSetFeeDestination_CREATE2BytecodeDetection_Locks() public {
|
||||
LiquidityManager freshLm = new LiquidityManager(address(factory), address(weth), address(harberg), address(optimizer));
|
||||
address eoaAddr = makeAddr("precomputedEOA");
|
||||
|
||||
// Step 1: set to EOA — allowed, lock stays false
|
||||
freshLm.setFeeDestination(eoaAddr);
|
||||
assertFalse(freshLm.feeDestinationLocked(), "not locked after EOA set");
|
||||
|
||||
// Step 2: simulate CREATE2 deploy — bytecode now exists at the fee destination address
|
||||
vm.etch(eoaAddr, hex"00");
|
||||
assertGt(eoaAddr.code.length, 0, "precondition: address has code");
|
||||
|
||||
// Step 3: calling setFeeDestination detects bytecode at the current destination and
|
||||
// commits feeDestinationLocked = true WITHOUT reverting, so the write survives
|
||||
// a later SELFDESTRUCT. feeDestination itself is not changed.
|
||||
freshLm.setFeeDestination(makeAddr("attacker"));
|
||||
assertTrue(freshLm.feeDestinationLocked(), "locked after bytecode detected at current feeDestination");
|
||||
assertEq(freshLm.feeDestination(), eoaAddr, "feeDestination must not change when defensive lock triggers");
|
||||
|
||||
// Step 4: simulate SELFDESTRUCT — bytecode gone, but storage lock persists
|
||||
vm.etch(eoaAddr, hex"");
|
||||
assertEq(eoaAddr.code.length, 0, "code cleared (selfdestruct simulation)");
|
||||
|
||||
// Step 5: re-assignment must still be blocked despite cleared bytecode
|
||||
vm.expectRevert("fee destination locked");
|
||||
freshLm.setFeeDestination(makeAddr("attacker"));
|
||||
}
|
||||
|
||||
/**
|
||||
* @notice When optimizer reverts, the catch block uses default bear params (covers lines 192-201)
|
||||
*/
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue