Skip to content

Commit

Permalink
allow permit2 to silently fail to avoid dos (#417)
Browse files Browse the repository at this point in the history
* allow permit2 to silently fail to avoid dos

* fix tests
  • Loading branch information
dianakocsis authored Nov 15, 2024
1 parent 1337089 commit 5aefc7e
Show file tree
Hide file tree
Showing 10 changed files with 143 additions and 53 deletions.
18 changes: 16 additions & 2 deletions contracts/base/Dispatcher.sol
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,14 @@ abstract contract Dispatcher is Payments, V2SwapRouter, V3SwapRouter, V4SwapRout
permitBatch := add(inputs.offset, calldataload(inputs.offset))
}
bytes calldata data = inputs.toBytes(1);
PERMIT2.permit(msgSender(), permitBatch, data);
(success, output) = address(PERMIT2).call(
abi.encodeWithSignature(
'permit(address,((address,uint160,uint48,uint48)[],address,uint256),bytes)',
msgSender(),
permitBatch,
data
)
);
} else if (command == Commands.SWEEP) {
// equivalent: abi.decode(inputs, (address, address, uint256))
address token;
Expand Down Expand Up @@ -185,7 +192,14 @@ abstract contract Dispatcher is Payments, V2SwapRouter, V3SwapRouter, V4SwapRout
permitSingle := inputs.offset
}
bytes calldata data = inputs.toBytes(6); // PermitSingle takes first 6 slots (0..5)
PERMIT2.permit(msgSender(), permitSingle, data);
(success, output) = address(PERMIT2).call(
abi.encodeWithSignature(
'permit(address,((address,uint160,uint48,uint48),address,uint256),bytes)',
msgSender(),
permitSingle,
data
)
);
} else if (command == Commands.WRAP_ETH) {
// equivalent: abi.decode(inputs, (address, uint256))
address recipient;
Expand Down
41 changes: 41 additions & 0 deletions test/integration-tests/UniswapMixed.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -279,6 +279,47 @@ describe('Uniswap V2, V3, and V4 Tests:', () => {
expect(wethBalanceAfter.sub(wethBalanceBefore)).to.be.gte(minAmountOut1.add(minAmountOut2))
})

it('PERMIT2 batch can silently fail', async () => {
const v2AmountIn1: BigNumber = expandTo18DecimalsBN(20)
const v2AmountIn2: BigNumber = expandTo18DecimalsBN(5)

const BATCH_PERMIT = {
details: [
{
token: DAI.address,
amount: v2AmountIn1,
expiration: 0, // expiration of 0 is block.timestamp
nonce: 0, // this is his first trade
},
{
token: WETH.address,
amount: v2AmountIn2,
expiration: 0, // expiration of 0 is block.timestamp
nonce: 0, // this is his first trade
},
],
spender: router.address,
sigDeadline: DEADLINE,
}

const sig = await getPermitBatchSignature(BATCH_PERMIT, bob, permit2)

// transfer funds into DAI-USDC and DAI-USDT pairs to trade
// do not allow revert
planner.addCommand(CommandType.PERMIT2_PERMIT_BATCH, [BATCH_PERMIT, sig])

// allow revert
planner.addCommand(CommandType.PERMIT2_PERMIT_BATCH, [BATCH_PERMIT, sig], true)

let nonce = (await permit2.allowance(bob.address, DAI.address, router.address)).nonce
expect(nonce).to.eq(0)

await executeRouter(planner, bob, router, wethContract, daiContract, usdcContract)

nonce = (await permit2.allowance(bob.address, DAI.address, router.address)).nonce
expect(nonce).to.eq(1)
})

it('ERC20 --> ERC20 split V2 and V2 different routes, different input tokens, each two hop, with batch permit', async () => {
const route1 = [DAI.address, WETH.address, USDC.address]
const route2 = [WETH.address, DAI.address, USDC.address]
Expand Down
31 changes: 31 additions & 0 deletions test/integration-tests/UniswapV2.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,37 @@ describe('Uniswap V2 Tests:', () => {
await permit2.approve(DAI.address, router.address, 0, 0)
})

it('Permit2 can silently fail', async () => {
const amountInDAI = expandTo18DecimalsBN(100)

// bob signs a permit to allow the router to access his DAI
permit = {
details: {
token: DAI.address,
amount: amountInDAI,
expiration: 0, // expiration of 0 is block.timestamp
nonce: 0, // this is his first trade
},
spender: router.address,
sigDeadline: DEADLINE,
}
const sig = await getPermitSignature(permit, bob, permit2)

// 1) permit the router to access funds, not allowing revert
planner.addCommand(CommandType.PERMIT2_PERMIT, [permit, sig])

// 2) permit the router to access funds again, allowing revert
planner.addCommand(CommandType.PERMIT2_PERMIT, [permit, sig], true)

let nonce = (await permit2.allowance(bob.address, DAI.address, router.address)).nonce
expect(nonce).to.eq(0)

await executeRouter(planner, bob, router, wethContract, daiContract, usdcContract)

nonce = (await permit2.allowance(bob.address, DAI.address, router.address)).nonce
expect(nonce).to.eq(1)
})

it('V2 exactIn, permiting the exact amount', async () => {
const amountInDAI = expandTo18DecimalsBN(100)
const minAmountOutWETH = expandTo18DecimalsBN(0.02)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,6 @@
exports[`Check Ownership Gas gas: balance check ERC20 1`] = `
Object {
"calldataByteLength": 356,
"gasUsed": 37698,
"gasUsed": 37740,
}
`;
Original file line number Diff line number Diff line change
Expand Up @@ -3,48 +3,48 @@
exports[`Payments Gas Tests Individual Command Tests gas: SWEEP with ERC20 1`] = `
Object {
"calldataByteLength": 356,
"gasUsed": 37012,
"gasUsed": 37042,
}
`;

exports[`Payments Gas Tests Individual Command Tests gas: SWEEP_WITH_FEE 1`] = `
Object {
"calldataByteLength": 516,
"gasUsed": 65726,
"gasUsed": 65786,
}
`;

exports[`Payments Gas Tests Individual Command Tests gas: TRANSFER with ERC20 1`] = `
Object {
"calldataByteLength": 356,
"gasUsed": 36040,
"gasUsed": 36070,
}
`;

exports[`Payments Gas Tests Individual Command Tests gas: TRANSFER with ETH 1`] = `
Object {
"calldataByteLength": 356,
"gasUsed": 31605,
"gasUsed": 31635,
}
`;

exports[`Payments Gas Tests Individual Command Tests gas: UNWRAP_WETH 1`] = `
Object {
"calldataByteLength": 324,
"gasUsed": 44579,
"gasUsed": 44621,
}
`;

exports[`Payments Gas Tests Individual Command Tests gas: UNWRAP_WETH_WITH_FEE 1`] = `
Object {
"calldataByteLength": 644,
"gasUsed": 51006,
"gasUsed": 51108,
}
`;

exports[`Payments Gas Tests Individual Command Tests gas: WRAP_ETH 1`] = `
Object {
"calldataByteLength": 324,
"gasUsed": 53389,
"gasUsed": 53423,
}
`;
Loading

0 comments on commit 5aefc7e

Please sign in to comment.