code-423n4 / 2021-11-vader-findings

0 stars 0 forks source link

add liquidity is vulnerable to sandwich attack #67

Open code423n4 opened 3 years ago

code423n4 commented 3 years ago

Handle

jonah1005

Vulnerability details

add liquidity is vulnerable to MEV

Impact

addLiquidity in the VaderRouter and VaderRouterV2 contract does not check the minimum liquidity amount. This makes users' funds vulnerable to sandwich attacks.

The team says a minimum amount is not required as the VaderPool supports imbalanced mint. However, imbalanced mint is a helper function of buying tokens and providing to lp. A sandwich attack would take over more than 50% of a transaction in an illiquid pool.

Given the current network environment, most transactions in the mempool would be sandwiched. However, users may avoid this attack if they only send tx through flashbot RPC. I consider this is a medium-risk issue.

Proof of Concept

VaderRouterV2.sol#L77-L96

That says a user wants to provide 1M ETH in the pool. Attackers can sandwich this trade as follows:

  1. Buy Vader with 10M ETH and makes ETH extremely cheap
  2. Put user's tx here User's tx would first buy a lot Vader and deposit to the pool.
  3. Since ETH becomes even cheaper in the pool. The MEV attacker buyback ETH and get profit.

Tools Used

None

Recommended Mitigation Steps

Always check how much liquidity a user received in a transaction. A tx would not be sandwiched if it's not profitable.

We could learn a lot about MEV from Robert Miller'tweets.

SamSteinGG commented 2 years ago

The design of the Thorchain CLP model is meant to prevent flash-loan based attacks as it allows a maximum trade size of 25% on a given iteration with a high-enough fee to render the attack unprofitable. Please request a tangible test case from the warden to consider this exhibit valid.

alcueca commented 2 years ago

There is no documentation stating that the deployment of Vader is restricted to Thorchain. The issue is valid.

SamSteinGG commented 2 years ago

@alcueca The model itself is what has this trait, it does not relate to the blockchain implementation itself. It is intended functionality as with its parent implementation.