Skip to content

Commit

Permalink
OZ-L06: try-catch permit2Forwarder to avoid exposing DOS on multicall (…
Browse files Browse the repository at this point in the history 
…#309)

* try-catch permit2Forwarder to avoid exposing DOS on multicall

* fix typo

* pr feedback nits

* return error and assert the permit error
  • Loading branch information
@saucepoint
saucepoint authored Sep 3, 2024
1 parent 9cb77ca commit e198c55
Show file tree
Hide file tree
Showing 2 changed files with 159 additions and 3 deletions.
16 changes: 14 additions & 2 deletions src/base/Permit2Forwarder.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@ import {IAllowanceTransfer} from "permit2/src/interfaces/IAllowanceTransfer.sol"
contract Permit2Forwarder {
IAllowanceTransfer public immutable permit2;

error Wrap__Permit2Reverted(address _permit2, bytes reason);

constructor(IAllowanceTransfer _permit2) {
permit2 = _permit2;
}
Expand All @@ -17,16 +19,26 @@ contract Permit2Forwarder {
function permit(address owner, IAllowanceTransfer.PermitSingle calldata permitSingle, bytes calldata signature)
external
payable
returns (bytes memory err)
{
permit2.permit(owner, permitSingle, signature);
// use try/catch in case an actor front-runs the permit, which would DOS multicalls
try permit2.permit(owner, permitSingle, signature) {}
catch (bytes memory reason) {
err = abi.encodeWithSelector(Wrap__Permit2Reverted.selector, address(permit2), reason);
}
}

/// @notice allows forwarding batch permits to permit2
/// @dev this function is payable to allow multicall with NATIVE based actions
function permitBatch(address owner, IAllowanceTransfer.PermitBatch calldata _permitBatch, bytes calldata signature)
external
payable
returns (bytes memory err)
{
permit2.permit(owner, _permitBatch, signature);
// use try/catch in case an actor front-runs the permit, which would DOS multicalls
try permit2.permit(owner, _permitBatch, signature) {}
catch (bytes memory reason) {
err = abi.encodeWithSelector(Wrap__Permit2Reverted.selector, address(permit2), reason);
}
}
}
146 changes: 145 additions & 1 deletion test/position-managers/PositionManager.multicall.t.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,8 @@ contract PositionManagerMulticallTest is Test, Permit2SignatureHelpers, PosmTest
address bob;
// bob used for permit2 signature tests
uint256 bobPK;
address charlie; // charlie will NOT approve posm in setup()
uint256 charliePK;

Permit2Forwarder permit2Forwarder;

Expand All @@ -54,13 +56,17 @@ contract PositionManagerMulticallTest is Test, Permit2SignatureHelpers, PosmTest
uint48 permitExpiration = uint48(block.timestamp + 10e18);
uint48 permitNonce = 0;

// redefine error from permit2/src/PermitErrors.sol since its hard-pinned to a solidity version
error InvalidNonce();

bytes32 PERMIT2_DOMAIN_SEPARATOR;

PositionConfig config;

function setUp() public {
(alice, alicePK) = makeAddrAndKey("ALICE");
(bob, bobPK) = makeAddrAndKey("BOB");
(charlie, charliePK) = makeAddrAndKey("CHARLIE");

deployFreshManagerAndRouters();
deployMintAndApprove2Currencies();
Expand All @@ -78,6 +84,13 @@ contract PositionManagerMulticallTest is Test, Permit2SignatureHelpers, PosmTest

seedBalance(bob);
approvePosmFor(bob);

// do not approve posm for charlie, but approve permit2 for allowance transfer
seedBalance(charlie);
vm.startPrank(charlie);
IERC20(Currency.unwrap(currency0)).approve(address(permit2), type(uint256).max);
IERC20(Currency.unwrap(currency1)).approve(address(permit2), type(uint256).max);
vm.stopPrank();
}

function test_multicall_initializePool_mint() public {
Expand Down Expand Up @@ -174,7 +187,6 @@ contract PositionManagerMulticallTest is Test, Permit2SignatureHelpers, PosmTest
bytes[] memory calls = new bytes[](1);
calls[0] = abi.encodeWithSelector(IPositionManager.modifyLiquidities.selector, actions, _deadline);

address charlie = makeAddr("CHARLIE");
vm.startPrank(charlie);
vm.expectRevert(abi.encodeWithSelector(IPositionManager.NotApproved.selector, charlie));
lpm.multicall(calls);
Expand Down Expand Up @@ -360,4 +372,136 @@ contract PositionManagerMulticallTest is Test, Permit2SignatureHelpers, PosmTest
assertEq(liquidity, 10e18);
assertEq(lpm.ownerOf(tokenId), bob);
}

/// @notice test that a front-ran permit does not fail a multicall with permit
function test_multicall_permit_frontrun_suceeds() public {
config = PositionConfig({
poolKey: key,
tickLower: TickMath.minUsableTick(key.tickSpacing),
tickUpper: TickMath.maxUsableTick(key.tickSpacing)
});

// Charlie signs permit for the two tokens
IAllowanceTransfer.PermitSingle memory permit0 =
defaultERC20PermitAllowance(Currency.unwrap(currency0), permitAmount, permitExpiration, permitNonce);
permit0.spender = address(lpm);
bytes memory sig0 = getPermitSignature(permit0, charliePK, PERMIT2_DOMAIN_SEPARATOR);

IAllowanceTransfer.PermitSingle memory permit1 =
defaultERC20PermitAllowance(Currency.unwrap(currency1), permitAmount, permitExpiration, permitNonce);
permit1.spender = address(lpm);
bytes memory sig1 = getPermitSignature(permit1, charliePK, PERMIT2_DOMAIN_SEPARATOR);

// bob front-runs the permits
vm.startPrank(bob);
lpm.permit(charlie, permit0, sig0);
lpm.permit(charlie, permit1, sig1);
vm.stopPrank();

// bob's front-run was successful
(uint160 _amount, uint48 _expiration, uint48 _nonce) =
permit2.allowance(charlie, Currency.unwrap(currency0), address(lpm));
assertEq(_amount, permitAmount);
assertEq(_expiration, permitExpiration);
assertEq(_nonce, permitNonce + 1);
(uint160 _amount1, uint48 _expiration1, uint48 _nonce1) =
permit2.allowance(charlie, Currency.unwrap(currency1), address(lpm));
assertEq(_amount1, permitAmount);
assertEq(_expiration1, permitExpiration);
assertEq(_nonce1, permitNonce + 1);

// charlie tries to mint an LP token with multicall(permit, permit, mint)
bytes[] memory calls = new bytes[](3);
calls[0] = abi.encodeWithSelector(Permit2Forwarder(lpm).permit.selector, charlie, permit0, sig0);
calls[1] = abi.encodeWithSelector(Permit2Forwarder(lpm).permit.selector, charlie, permit1, sig1);
bytes memory mintCall = getMintEncoded(config, 10e18, charlie, ZERO_BYTES);
calls[2] = abi.encodeWithSelector(IPositionManager.modifyLiquidities.selector, mintCall, _deadline);

uint256 tokenId = lpm.nextTokenId();
vm.expectRevert();
lpm.ownerOf(tokenId); // token does not exist

bytes[] memory results = lpm.multicall(calls);
assertEq(
results[0],
abi.encode(
abi.encodeWithSelector(
Permit2Forwarder.Wrap__Permit2Reverted.selector,
address(permit2),
abi.encodeWithSelector(InvalidNonce.selector)
)
)
);
assertEq(
results[1],
abi.encode(
abi.encodeWithSelector(
Permit2Forwarder.Wrap__Permit2Reverted.selector,
address(permit2),
abi.encodeWithSelector(InvalidNonce.selector)
)
)
);

assertEq(lpm.ownerOf(tokenId), charlie);
}

/// @notice test that a front-ran permitBatch does not fail a multicall with permitBatch
function test_multicall_permitBatch_frontrun_suceeds() public {
config = PositionConfig({
poolKey: key,
tickLower: TickMath.minUsableTick(key.tickSpacing),
tickUpper: TickMath.maxUsableTick(key.tickSpacing)
});

// Charlie signs permitBatch for the two tokens
address[] memory tokens = new address[](2);
tokens[0] = Currency.unwrap(currency0);
tokens[1] = Currency.unwrap(currency1);

IAllowanceTransfer.PermitBatch memory permit =
defaultERC20PermitBatchAllowance(tokens, permitAmount, permitExpiration, permitNonce);
permit.spender = address(lpm);
bytes memory sig = getPermitBatchSignature(permit, charliePK, PERMIT2_DOMAIN_SEPARATOR);

// bob front-runs the permits
vm.prank(bob);
lpm.permitBatch(charlie, permit, sig);

// bob's front-run was successful
(uint160 _amount, uint48 _expiration, uint48 _nonce) =
permit2.allowance(charlie, Currency.unwrap(currency0), address(lpm));
assertEq(_amount, permitAmount);
assertEq(_expiration, permitExpiration);
assertEq(_nonce, permitNonce + 1);
(uint160 _amount1, uint48 _expiration1, uint48 _nonce1) =
permit2.allowance(charlie, Currency.unwrap(currency1), address(lpm));
assertEq(_amount1, permitAmount);
assertEq(_expiration1, permitExpiration);
assertEq(_nonce1, permitNonce + 1);

// charlie tries to mint an LP token with multicall(permitBatch, mint)
bytes[] memory calls = new bytes[](2);
calls[0] = abi.encodeWithSelector(Permit2Forwarder(lpm).permitBatch.selector, charlie, permit, sig);
bytes memory mintCall = getMintEncoded(config, 10e18, charlie, ZERO_BYTES);
calls[1] = abi.encodeWithSelector(IPositionManager.modifyLiquidities.selector, mintCall, _deadline);

uint256 tokenId = lpm.nextTokenId();
vm.expectRevert();
lpm.ownerOf(tokenId); // token does not exist

bytes[] memory results = lpm.multicall(calls);
assertEq(
results[0],
abi.encode(
abi.encodeWithSelector(
Permit2Forwarder.Wrap__Permit2Reverted.selector,
address(permit2),
abi.encodeWithSelector(InvalidNonce.selector)
)
)
);

assertEq(lpm.ownerOf(tokenId), charlie);
}
}

0 comments on commit e198c55

Please sign in to comment.