code-423n4 / 2023-01-numoen-findings

0 stars 0 forks source link

Multiple combinations of token0/token1 for a given liquidity exist to satisfies the custom variant of AMM pool. A naive LP or Power token holder can transfer more token0/token1 then necessary when minting & burning respectively #244

Closed code423n4 closed 1 year ago

code423n4 commented 1 year ago

Lines of code

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

Vulnerability details

Impact

LP's need to provide a combination of token0 / token 1 for a given liquidity that satisfied a custom variant that satisfies 2 conditions

1. scale1 < 2 * upperBound

2. a + b >= c + d (where a, b, c, d are functions of token0/ token1 , liquidity and upper bound)

The relationship between these variables is non-linear & for a given liquidity, there exists multiple combinations of token0/token1 amounts that satisfy the custom invariant. While testing, I found it difficult to come with correct guess values & I assume it would be very tough for LPs and power token holders to guess the token0/token1 combo.

There is a possibility of users depositing more of token 0 / token 1 into the pool for a given level of liquidity.

Proof of Concept

For existing Power token holder with following values:

token 0 = 0.6 ether, token 1 = 4.8 ether, liquidity borrowed = 0.118668 ether, share = 0.82,

for burning 0.5 shares, I could find 2 combinations of token 0 /token 1 => 0.5 ether/ 0.8 ether and 0.3 ether/ 0.6 ether that satisfy custom invariant. As a naive power token holder, I could choose 1 option over other & might deposit higher tokens than necessary when burning power tokens.

Tools Used

Manual

Recommended Mitigation Steps

Since the returns & IL for LPs and the borrowing cost for power token holders depend on token0/token1 holdings, there must be an optimal token distribution for LPs and token holders. Protocol should implement a function that calculates the optimal token 0 & token 1 for LP holder and power token holders in the LiquidityManager wrapper. Any tokens transferred in excess of this optimal values can be returned back to the payer

c4-sponsor commented 1 year ago

kyscott18 marked the issue as sponsor disputed

kyscott18 commented 1 year ago

When determining the optimal amounts to deposit and withdraw, there are two main cases. When the pool already contains some liquidity, it is trivial to determine the amounts by using proportional amounts to what is already deposited in the pool. This will result in the highest value for liquidity providers. When pools are empty, amounts of token0 and token1 can be determined from a price P that will be the quote price for the liquidity pool after deposit.

c4-judge commented 1 year ago

berndartmueller marked the issue as unsatisfactory: Invalid