code423n4 commented 1 year ago

Lines of code

https://github.com/code-423n4/2023-01-numoen/blob/2ad9a73d793ea23a25a381faadc86ae0c8cb5913/src/core/Lendgine.sol#L83-L93

Vulnerability details

Impact

A malicious early user/attacker can manipulate the share price to take an unfair share of future users' deposits.

The first minter can manipulate the supply of LP tokens and baseToken-fractional ratio, hindering small liquidity providers from interacting with the pair.

A malicious actor can mint 1wei of LP token from a new pair, then proceed to transfer baseToken to the Pair contract, artificially increasing the baseToken reserves. Therefore 1 wei of LP token can be worth arbitrarily large amounts of baseToken and fractional token (depending on how much baseToken is deposited to the Pair)

Proof of Concept

When adding liquidity, this code is called: https://github.com/code-423n4/2023-01-numoen/blob/2ad9a73d793ea23a25a381faadc86ae0c8cb5913/src/core/Lendgine.sol#L146

mint(liquidity, data);

Which calls: https://github.com/code-423n4/2023-01-numoen/blob/2ad9a73d793ea23a25a381faadc86ae0c8cb5913/src/core/Lendgine.sol#L83-L93

uint256 liquidity = convertCollateralToLiquidity(collateral);
shares = convertLiquidityToShare(liquidity);

if (collateral == 0 || liquidity == 0 || shares == 0) revert InputError();
if (liquidity > totalLiquidity) revert CompleteUtilizationError();
// next check is for the case when liquidity is borrowed but then was completely accrued
if (totalSupply > 0 && totalLiquidityBorrowed == 0) revert CompleteUtilizationError();

totalLiquidityBorrowed += liquidity;
(uint256 amount0, uint256 amount1) = burn(to, liquidity);
_mint(to, shares);

Pair.sol and Lendgine.sol contract uses the same Maths applied in Uniswap v2. When the totalSupply of the LP token is zero, this means the LP provider is the first and thus initially, the contract mints shares equal to the geometric mean of the amounts deposited. In this case, that would be amount0 and amount1 provided by the user. However, there is a risk due to the increase of value of the LP token, either through fees or “donations” directly sent in that could make the value of the smallest unit of the LP token(1e-18) so expensive that it is not feasible for small liquidity providers to provide as the LP token they would getting after looking up their tokens is negligible. To avoid this, Uniswap v2 deducts (1e-15) which is 1000 times the smallest unit of the LP token(1e-18) during initial mint to make this type of attack substantially more difficult. However, in caviar Lendgine.sol, deposit() simply returns the geometric mean of the tokens provided without any initial deductions.

Tools Used

Manual Review

Recommended Mitigation Steps

Please consider minting a minimal amount of LP tokens during the first mint and sending them to zero address, this increases the cost of the attack. Uniswap V2 uses the value 1000 as it is small enough to don't hurt the first minter, while still increasing the cost of this attack by 1000x. https://github.com/Uniswap/v2-core/blob/master/contracts/UniswapV2Pair.sol#L119-L124

c4-judge commented 1 year ago

berndartmueller marked the issue as duplicate of #32