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

0 stars 0 forks source link

Early user can break `addLiquidity` #189

Open code423n4 opened 2 years ago

code423n4 commented 2 years ago

Handle

WatchPug

Vulnerability details

https://github.com/code-423n4/2021-11-vader/blob/429970427b4dc65e37808d7116b9de27e395ce0c/contracts/dex/pool/BasePool.sol#L161-L163

uint256 totalLiquidityUnits = totalSupply;
if (totalLiquidityUnits == 0)
    liquidity = nativeDeposit; // TODO: Contact ThorChain on proper approach

In the current implementation, the first liquidity takes the nativeDeposit amount and uses it directly.

However, since this number (totalLiquidityUnits) will later be used for computing the liquidity issued for future addLiquidity using calculateLiquidityUnits.

A malicious user can add liquidity with only 1 wei USDV and making it nearly impossible for future users to add liquidity to the pool.

Recomandation

Uni v2 solved this problem by sending the first 1000 tokens to the zero address.

The same should work here, i.e., on first mint (totalLiquidityUnits == 0), lock some of the first minter's tokens by minting ~1% of the initial amount to the zero address instead of to the first minter.

SamSteinGG commented 2 years ago

Duplicate of #24

alcueca commented 2 years ago

Not a duplicate