Vanguard
First Flight #56
Beginner FriendlyDeFiFoundry
0 EXP
View all submissions
Previous Next
Submission Details
Impact: medium
Likelihood: medium

Inaccurate swap amount accounting, causing misapplied limits/penalties for users.

Author Revealed upon completion

Root + Impact

Description

  • Uniswap has several parameters that indicate swap direction zeroForOne and positive/negative amountSpecified. Negative amountSpecified represents the exact amount of token0 (or token1 in case zeroForOne=false) the user is willing to spend for a swap, and positive amountSpecified represents how much of token1 (or token0 in case zeroForOne=true) the user wants to receive. Both negative and positive amountSpecified provide amounts in token-specific units, which can cause incorrect calculation of swapped amount tracking and cause users additional penalties.

  • However, in the _beforeSwap function, the swapAmount calculation treats negative and positive amountSpecified equally, adjusting both to positive values. Meaning, in the case of exact input for output it counts the correct swap amount, but in the case of exact output for input it counts the other token amount, not the actual spend amount.

function _beforeSwap(address sender, PoolKey calldata key, SwapParams calldata params, bytes calldata)
internal
override
returns (bytes4, BeforeSwapDelta, uint24)
{
.
.
.
uint256 swapAmount =
@> params.amountSpecified < 0 ? uint256(-params.amountSpecified) : uint256(params.amountSpecified);
uint256 maxSwapAmount = (initialLiquidity * phaseLimitBps) / 10000;
.
.
.
}

Risk

Likelihood:

  • Does not require specific actions, standard swap flow for exact input for output, and exact output for input.

  • Occurs on any swap; the swap amount is just added to a storage slot to be used for penalty calculation later in the protocol functionality.

Impact:

  • Can force users to hit phase limits by accounting for different token-specific amounts, or vice versa prevent users and bots from reaching limits.

Proof of Concept

This test proves that the recorded swap amount in addressSwappedAmount does not account for correct swap amounts.

Add this snippet of code to test/TokenLaunchHookUnit.t.sol

function test_POC_ExactOutput_SwapAmount_Misaccounted() public {
uint256 amountOutToken = 1_000_000; // 1 token with 6 decimals
​
vm.deal(user1, 1 ether);
vm.startPrank(user1);
​
SwapParams memory params = SwapParams({
zeroForOne: true, amountSpecified: int256(amountOutToken), sqrtPriceLimitX96: TickMath.MIN_SQRT_PRICE + 1
});
​
PoolSwapTest.TestSettings memory testSettings =
PoolSwapTest.TestSettings({takeClaims: false, settleUsingBurn: false});
​
uint256 recordedBefore = antiBotHook.addressSwappedAmount(address(swapRouter));
BalanceDelta delta = swapRouter.swap{value: 1 ether}(key, params, testSettings, ZERO_BYTES);
uint256 recordedAfter = antiBotHook.addressSwappedAmount(address(swapRouter));
​
uint256 recordedSwapAmount = recordedAfter - recordedBefore;
uint256 actualInputEth = uint256(int256(-delta.amount0()));
​
assertEq(recordedSwapAmount, amountOutToken, "Recorded swap amount uses amountSpecified");
assertTrue(actualInputEth != amountOutToken, "Actual input differs from recorded amount");
​
console.log("actualInputEth: ",actualInputEth);
console.log("recordedSwapAmount: ",recordedSwapAmount);
​
vm.stopPrank();
}

Recommended Mitigation

Adjust addressSwappedAmount based on the afterSwap BalanceDelta. It would not fully remove the risk of miscalculation, but it would definitely reduce the number of negative scenarios.

function _beforeSwap(address sender, PoolKey calldata key, SwapParams calldata params, bytes calldata)
internal
override
returns (bytes4, BeforeSwapDelta, uint24)
{
.
.
.
- addressSwappedAmount[sender] += swapAmount;
- addressLastSwapBlock[sender] = block.number;
+ if(params.amountSpecified < 0) {
+ addressSwappedAmount[sender] += swapAmount;
+ addressLastSwapBlock[sender] = block.number;
}
.
.
.
}
.
.
.
+ function _afterSwap(address, PoolKey calldata, SwapParams calldata, BalanceDelta delta, bytes calldata)
+ internal
+ virtual
+ returns (bytes4, int128)
+ {
+ if(params.amountSpecified > 0) {
+ addressSwappedAmount[sender] += delta.amount1();
+ addressLastSwapBlock[sender] = block.number;
+ }
+ }

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.

Give us feedback!