From aedec63f812b2d63a83387a7c72de93d72218d1a Mon Sep 17 00:00:00 2001 From: tapakornl Date: Sat, 9 Mar 2024 16:24:18 +0700 Subject: [PATCH 1/6] fix bug: frontrun permit --- contracts/src/gas-swap/GasSwap.sol | 23 +++++++++++++---------- 1 file changed, 13 insertions(+), 10 deletions(-) diff --git a/contracts/src/gas-swap/GasSwap.sol b/contracts/src/gas-swap/GasSwap.sol index 7b3800f417..1657de92c9 100644 --- a/contracts/src/gas-swap/GasSwap.sol +++ b/contracts/src/gas-swap/GasSwap.sol @@ -14,7 +14,6 @@ import {Context} from "@openzeppelin/contracts/utils/Context.sol"; contract GasSwap is ERC2771Context, Ownable, ReentrancyGuard { using SafeERC20 for IERC20; - using SafeERC20 for IERC20Permit; /********** * Events * @@ -92,15 +91,19 @@ contract GasSwap is ERC2771Context, Ownable, ReentrancyGuard { address _sender = _msgSender(); // do permit - IERC20Permit(_permit.token).safePermit( - _sender, - address(this), - _permit.value, - _permit.deadline, - _permit.v, - _permit.r, - _permit.s - ); + try + IERC20Permit(_permit.token).permit( + _sender, + address(this), + _permit.value, + _permit.deadline, + _permit.v, + _permit.r, + _permit.s + ) + {} catch { + require(IERC20(_permit.token).allowance(_sender, address(this)) >= _permit.value, "Permit failed"); + } // record token balance in this contract uint256 _balance = IERC20(_permit.token).balanceOf(address(this)); From 047f5d17eaded79f95d97e323ffbd392cf34cd0f Mon Sep 17 00:00:00 2001 From: tapakornl Date: Sat, 9 Mar 2024 16:30:55 +0700 Subject: [PATCH 2/6] test gasSwap frontrun permit --- contracts/integration-test/GasSwap.spec.ts | 40 ++++++++++++++++++++++ 1 file changed, 40 insertions(+) diff --git a/contracts/integration-test/GasSwap.spec.ts b/contracts/integration-test/GasSwap.spec.ts index 49c3b33d31..69f6a1bd07 100644 --- a/contracts/integration-test/GasSwap.spec.ts +++ b/contracts/integration-test/GasSwap.spec.ts @@ -197,6 +197,46 @@ describe("GasSwap.spec", async () => { ).to.revertedWith("insufficient output amount"); }); + it("should succeed, when attacker frontrun permit", async () => { + const amountIn = ethers.utils.parseEther("1"); + const amountOut = ethers.utils.parseEther("2"); + await token.mint(signer.address, amountIn); + await deployer.sendTransaction({ to: target.address, value: amountOut }); + const signature = await permit(amountIn); + + await token.permit( + signer.address, + swap.address, + amountIn, + constants.MaxUint256, + signature.v, + signature.r, + signature.s + ); + + await target.setToken(token.address); + await target.setAmountIn(amountIn); + + await swap.updateApprovedTarget(target.address, true); + await expect( + swap.connect(signer).swap( + { + token: token.address, + value: amountIn, + deadline: constants.MaxUint256, + r: signature.r, + s: signature.s, + v: signature.v, + }, + { + target: target.address, + data: "0x8119c065", + minOutput: 0, + } + ) + ); + }); + for (const refundRatio of ["0", "1", "5"]) { for (const feeRatio of ["0", "5", "50"]) { it(`should succeed, when swap by signer directly, with feeRatio[${feeRatio}%] refundRatio[${refundRatio}%]`, async () => { From c882e8c0d3b1dfcec30dba2102e59951885c9f62 Mon Sep 17 00:00:00 2001 From: tapakornl Date: Wed, 13 Mar 2024 00:12:01 +0700 Subject: [PATCH 3/6] rm expect --- contracts/integration-test/GasSwap.spec.ts | 31 +++++++++++----------- 1 file changed, 15 insertions(+), 16 deletions(-) diff --git a/contracts/integration-test/GasSwap.spec.ts b/contracts/integration-test/GasSwap.spec.ts index 69f6a1bd07..d86b1828f2 100644 --- a/contracts/integration-test/GasSwap.spec.ts +++ b/contracts/integration-test/GasSwap.spec.ts @@ -218,22 +218,21 @@ describe("GasSwap.spec", async () => { await target.setAmountIn(amountIn); await swap.updateApprovedTarget(target.address, true); - await expect( - swap.connect(signer).swap( - { - token: token.address, - value: amountIn, - deadline: constants.MaxUint256, - r: signature.r, - s: signature.s, - v: signature.v, - }, - { - target: target.address, - data: "0x8119c065", - minOutput: 0, - } - ) + + swap.connect(signer).swap( + { + token: token.address, + value: amountIn, + deadline: constants.MaxUint256, + r: signature.r, + s: signature.s, + v: signature.v, + }, + { + target: target.address, + data: "0x8119c065", + minOutput: 0, + } ); }); From de755cca58c2cc485b7bc661a75824cfe12ad16e Mon Sep 17 00:00:00 2001 From: tapakornl Date: Sat, 9 Mar 2024 16:24:18 +0700 Subject: [PATCH 4/6] fix bug: frontrun permit --- contracts/src/gas-swap/GasSwap.sol | 23 +++++++++++++---------- 1 file changed, 13 insertions(+), 10 deletions(-) diff --git a/contracts/src/gas-swap/GasSwap.sol b/contracts/src/gas-swap/GasSwap.sol index 7b3800f417..1657de92c9 100644 --- a/contracts/src/gas-swap/GasSwap.sol +++ b/contracts/src/gas-swap/GasSwap.sol @@ -14,7 +14,6 @@ import {Context} from "@openzeppelin/contracts/utils/Context.sol"; contract GasSwap is ERC2771Context, Ownable, ReentrancyGuard { using SafeERC20 for IERC20; - using SafeERC20 for IERC20Permit; /********** * Events * @@ -92,15 +91,19 @@ contract GasSwap is ERC2771Context, Ownable, ReentrancyGuard { address _sender = _msgSender(); // do permit - IERC20Permit(_permit.token).safePermit( - _sender, - address(this), - _permit.value, - _permit.deadline, - _permit.v, - _permit.r, - _permit.s - ); + try + IERC20Permit(_permit.token).permit( + _sender, + address(this), + _permit.value, + _permit.deadline, + _permit.v, + _permit.r, + _permit.s + ) + {} catch { + require(IERC20(_permit.token).allowance(_sender, address(this)) >= _permit.value, "Permit failed"); + } // record token balance in this contract uint256 _balance = IERC20(_permit.token).balanceOf(address(this)); From ca0986e86844cfa059b8cfcd04b0456abbecc349 Mon Sep 17 00:00:00 2001 From: tapakornl Date: Sat, 9 Mar 2024 16:30:55 +0700 Subject: [PATCH 5/6] test gasSwap frontrun permit --- contracts/integration-test/GasSwap.spec.ts | 40 ++++++++++++++++++++++ 1 file changed, 40 insertions(+) diff --git a/contracts/integration-test/GasSwap.spec.ts b/contracts/integration-test/GasSwap.spec.ts index 49c3b33d31..69f6a1bd07 100644 --- a/contracts/integration-test/GasSwap.spec.ts +++ b/contracts/integration-test/GasSwap.spec.ts @@ -197,6 +197,46 @@ describe("GasSwap.spec", async () => { ).to.revertedWith("insufficient output amount"); }); + it("should succeed, when attacker frontrun permit", async () => { + const amountIn = ethers.utils.parseEther("1"); + const amountOut = ethers.utils.parseEther("2"); + await token.mint(signer.address, amountIn); + await deployer.sendTransaction({ to: target.address, value: amountOut }); + const signature = await permit(amountIn); + + await token.permit( + signer.address, + swap.address, + amountIn, + constants.MaxUint256, + signature.v, + signature.r, + signature.s + ); + + await target.setToken(token.address); + await target.setAmountIn(amountIn); + + await swap.updateApprovedTarget(target.address, true); + await expect( + swap.connect(signer).swap( + { + token: token.address, + value: amountIn, + deadline: constants.MaxUint256, + r: signature.r, + s: signature.s, + v: signature.v, + }, + { + target: target.address, + data: "0x8119c065", + minOutput: 0, + } + ) + ); + }); + for (const refundRatio of ["0", "1", "5"]) { for (const feeRatio of ["0", "5", "50"]) { it(`should succeed, when swap by signer directly, with feeRatio[${feeRatio}%] refundRatio[${refundRatio}%]`, async () => { From 2a96c8fa032a235735e06795843923b3d1c55f5b Mon Sep 17 00:00:00 2001 From: tapakornl Date: Wed, 13 Mar 2024 00:12:01 +0700 Subject: [PATCH 6/6] rm expect --- contracts/integration-test/GasSwap.spec.ts | 31 +++++++++++----------- 1 file changed, 15 insertions(+), 16 deletions(-) diff --git a/contracts/integration-test/GasSwap.spec.ts b/contracts/integration-test/GasSwap.spec.ts index 69f6a1bd07..d86b1828f2 100644 --- a/contracts/integration-test/GasSwap.spec.ts +++ b/contracts/integration-test/GasSwap.spec.ts @@ -218,22 +218,21 @@ describe("GasSwap.spec", async () => { await target.setAmountIn(amountIn); await swap.updateApprovedTarget(target.address, true); - await expect( - swap.connect(signer).swap( - { - token: token.address, - value: amountIn, - deadline: constants.MaxUint256, - r: signature.r, - s: signature.s, - v: signature.v, - }, - { - target: target.address, - data: "0x8119c065", - minOutput: 0, - } - ) + + swap.connect(signer).swap( + { + token: token.address, + value: amountIn, + deadline: constants.MaxUint256, + r: signature.r, + s: signature.s, + v: signature.v, + }, + { + target: target.address, + data: "0x8119c065", + minOutput: 0, + } ); });