Platform: Code4rena
Start Date: 30/05/2023
Pot Size: $300,500 USDC
Total HM: 79
Participants: 101
Period: about 1 month
Judge: Trust
Total Solo HM: 36
Id: 242
League: ETH
Rank: 91/101
Findings: 2
Award: $24.76
π Selected for report: 0
π Solo Findings: 0
π Selected for report: Madalad
Also found by: 0xCiphky, 0xSmartContract, 8olidity, BPZ, Breeje, BugBusters, Kaiziron, MohammedRizwan, Oxsadeeq, Qeew, RED-LOTUS-REACH, T1MOH, brgltd, chaduke, giovannidisiena, lsaudit, peanuts, tsvetanovv
10.4044 USDC - $10.40
The presence of the front-running vulnerability in the _compoundFees
function can have significant repercussions for users. Exploiting this vulnerability enables malicious actors to prioritize their transactions, potentially leading to unfavorable prices for other users. Consequently, users who initially submitted their transactions may end up receiving worse prices than they anticipated. This can result in financial losses.
The _compoundFees
function in the TalosStrategyVanilla.sol
code is responsible for increasing liquidity in a pool. However, it suffers from a front-running vulnerability due to the inadequate specification of minimum token amounts (amount0Min and amount1Min).
By setting both amount0Min
and amount1Min
to 0, the code allows any amount of token0 and token1 to be accepted during the liquidity increase. This lack of minimum token amount validation creates a potential security vulnerability that can be exploited by malicious users.
if (_liquidity > 0) { uint128 liquidityDifference; (liquidityDifference, amount0, amount1) = nonfungiblePositionManager.increaseLiquidity( INonfungiblePositionManager.IncreaseLiquidityParams({ tokenId: _tokenId, amount0Desired: balance0, amount1Desired: balance1, amount0Min: 0, amount1Min: 0, deadline: block.timestamp }) );
When the amount0Min and amount1Min parameters are both set to 0, the code allows any amount of token0 and token1 to be accepted during the liquidity increase. This lack of minimum token amount validation creates an opportunity for attackers to manipulate the transaction and impact the price received by the victim.
Malicious users can monitor pending transactions and quickly execute their own transaction with higher fees, effectively "sandwiching" the original transaction. By manipulating the price, the attacker can cause the original transaction to receive unfavorable rates, resulting in financial harm to the user.
Manual Review
To address the front-running vulnerability and protect users from receiving unfavorable prices, the following mitigation steps are recommended:
Implement Minimum Return Amounts: Introduce a mechanism that ensures users receive their desired minimum amounts of token0 and token1 during liquidity increases. By setting appropriate minimum return amounts, users can be protected from receiving significantly less than their expected values.
Set Minimum Amounts for Liquidity Provisions/Removals: Establish minimum acceptable amounts of token0 and token1 for liquidity provisions and removals. This ensures that users maintain a reasonable level of control over the transaction outcomes and prevents front-running attacks from manipulating the prices.
MEV
#0 - c4-judge
2023-07-09T15:31:45Z
trust1995 marked the issue as primary issue
#1 - c4-judge
2023-07-09T15:31:49Z
trust1995 marked the issue as satisfactory
#2 - c4-judge
2023-07-11T17:01:17Z
trust1995 changed the severity to 3 (High Risk)
#3 - c4-judge
2023-07-11T17:04:13Z
trust1995 changed the severity to 2 (Med Risk)
#4 - c4-judge
2023-07-11T17:04:22Z
trust1995 changed the severity to 3 (High Risk)
#5 - c4-sponsor
2023-07-13T11:42:13Z
0xLightt marked the issue as sponsor acknowledged
#6 - c4-sponsor
2023-07-13T11:50:25Z
0xLightt marked the issue as disagree with severity
#7 - 0xLightt
2023-07-13T11:50:27Z
This is intended here, in order to reach this the checkDeviation
modifier needs to pass, already offering some level of protection. Adding slippage protection here could be dangerous because it could potentially brick users' funds, this is for auto-compounding rewards.
#8 - c4-judge
2023-07-25T08:54:05Z
trust1995 changed the severity to 2 (Med Risk)
#9 - c4-judge
2023-07-25T13:46:05Z
trust1995 marked issue #577 as primary and marked this issue as a duplicate of 577
π Selected for report: 0xnev
Also found by: 0xSmartContract, Breeje, BugBusters, ByteBandits, IllIllI, Kaiziron, Madalad, MohammedRizwan, SpicyMeatball, T1MOH, kutugu, nadin, peanuts, said, shealtielanz, tsvetanovv
14.356 USDC - $14.36
Not allowing users to supply their own deadline could potentially expose them to sandwich attacks
function _compoundFees(uint256 _tokenId) internal returns (uint256 amount0, uint256 amount1) { uint256 balance0 = token0.balanceOf(address(this)) - protocolFees0; uint256 balance1 = token1.balanceOf(address(this)) - protocolFees1; emit Snapshot(balance0, balance1); //Get Liquidity for Optimizer's balances uint128 _liquidity = pool.liquidityForAmounts(balance0, balance1, tickLower, tickUpper); // Add liquidity to the pool if (_liquidity > 0) { uint128 liquidityDifference; (liquidityDifference, amount0, amount1) = nonfungiblePositionManager.increaseLiquidity( INonfungiblePositionManager.IncreaseLiquidityParams({ tokenId: _tokenId, amount0Desired: balance0, amount1Desired: balance1, amount0Min: 0, amount1Min: 0, deadline: block.timestamp //@audit no deadline }) ); liquidity += liquidityDifference; emit CompoundFees(amount0, amount1); } }
In _compoundFees
function of TalosStrategyVanilla.sol
, User is providing liquidity to the pool but it does not allow user to include a deadline check and it is hardcoded to block.timestamp
.
deadline: block.timestamp
In _compoundFees
function of TalosStrategyVanilla.sol
, user is providing liquidity to the pool but the deadline parameter is simply passed in as current block.timestamp in which transaction occurs. This effectively means that transaction has no deadline, which means that the transaction may be included anytime by validators and remain pending in mempool, potentially exposing users to sandwich attacks by attackers or MEV bots.
Consider the following scenario:
Alice wants to provide liquidity 300 BNB token for 30 ETH and later sell the 1 ETH for 3000 DAI. She signs the transaction.
The transaction is submitted to the mempool, however, Alice chose a transaction fee that is too low for validators to be interested in including her transaction in a block. The transaction stays pending in the mempool for extended periods, which could be hours, days, weeks, or even longer.
When the average gas fee dropped far enough for Alice's transaction to become interesting again for miners to include it, her trade will be executed. In the meantime, the price of ETH could have drastically decreased and the DAI value of that output might be significantly lower. She has unknowingly performed a bad trade due to the pending transaction she forgot about.
An even worse way this issue can be maliciously exploited is through MEV:
The transaction is still pending in the mempool. Average fees are still too high for validators to be interested in it. The price of ETH has gone up significantly since the transaction was signed, meaning Alice would receive a lot more when the trade is executed.
Allow users to supply their own deadline parameter within In _compoundFees
function of TalosStrategyVanilla.sol
Other
#0 - c4-judge
2023-07-09T11:16:26Z
trust1995 marked the issue as primary issue
#1 - c4-judge
2023-07-09T15:25:43Z
trust1995 marked the issue as satisfactory
#2 - c4-sponsor
2023-07-13T11:38:29Z
0xLightt marked the issue as sponsor acknowledged
#3 - c4-sponsor
2023-07-13T11:41:47Z
0xLightt marked the issue as sponsor confirmed
#4 - c4-judge
2023-07-25T13:48:16Z
trust1995 marked issue #504 as primary and marked this issue as a duplicate of 504