code-423n4 / 2024-03-revert-lend-findings

13 stars 10 forks source link

Malicous user can front-run the execute function in Autorange causing user transfrom call to always fail #124

Open c4-bot-10 opened 8 months ago

c4-bot-10 commented 8 months ago

Lines of code

https://github.com/code-423n4/2024-03-revert-lend/blob/main/src/transformers/AutoRange.sol#L133-L135

Vulnerability details

Impact

Anyone can add liquidity for anyone in uniswap ( no need to be the owner of the token ). By this, the malicious user can wait for the user who is calling the transform function to execute in the autorange and add dust liquidity to his uniswap position causing it to always revert, because of this if

 if (state.liquidity != params.liquidity) {
     revert LiquidityChanged();
 }

Proof of Concept

  1. User A want to calls the autorange execute function.
  2. So he calls transform passing the params in data.
  3. User B notices that user A is calling the execute, so he front-run it and sends dust liquidity to his position in uniswap.
  4. It reaches this if in the execute and notices different liquidity so it reverts.

Tools Used

Vscode

Recommended Mitigation Steps

Fix this if so that the function does not revert.

Assessed type

DoS

c4-pre-sort commented 8 months ago

0xEVom marked the issue as primary issue

c4-pre-sort commented 8 months ago

0xEVom marked the issue as sufficient quality report

c4-sponsor commented 8 months ago

kalinbas (sponsor) acknowledged

kalinbas commented 8 months ago

We are aware of this. Only operator accounts can call these functions and our bots monitor failed transactions. But its a valid finding as it may lead to some reverted tx for our bots. But because the check is at the beginning it helps to lower the gas cost for these reverted tx.

jhsagd76 commented 8 months ago

Like the issues with liquidations, there is no real point to this attack, as it neither profits nor truly halts the protocol. It only causes a transaction from an operator to fail and donates a minuscule amount of assets to the protocol. Therefore, I am downgrading it to low.

Additionally, why don't we just have the operator use an RPC with mempool protection? Even though this would slightly increase gas costs.

c4-judge commented 8 months ago

jhsagd76 changed the severity to QA (Quality Assurance)

c4-judge commented 8 months ago

jhsagd76 marked the issue as grade-a