First liquidity provider will suffer from revert or fund loss #254
Labels
2 (Med Risk)
Assets not at direct risk, but function/availability of the protocol could be impacted or leak value
bug
Something isn't working
M-02
satisfactory
satisfies C4 submission criteria; eligible for awards
selected for report
This submission will be included/highlighted in the audit report
sponsor acknowledged
Technically the issue is correct, but we're not going to resolve it for XYZ reasons
Lines of code
https://github.com/code-423n4/2023-01-numoen/blob/2ad9a73d793ea23a25a381faadc86ae0c8cb5913/src/periphery/LiquidityManager.sol#L135
Vulnerability details
Impact
The first liquidity depositor should supply three input values
amount0Min, amount1Min, liquidity
viaAddLiquidityParams
but these three values should meet an accurate relationship, or else the depositor will suffer from revert or fund lossProof of Concept
The LPs are supposed to use the function
LiquidityManager.addLiquidity(AddLiquidityParams calldata params)
to add liquidity.When the pool is not empty, this function calculates the
amount0, amount1
according to the current total liquidity and the requested liquidity.But when the pool is empty, these amounts are supposed to be provided by the caller.
Then how does the caller decides these amount? These values should be chosen very carefully as we explain below.
The whole protocol is based on its invariant that is defined in
Pair.invariant()
.The invariant is actually ensuring that
a+b-c-d
stays not negative for all trades (interactions regarding reserve/liquidity).Once
a+b-c-d
becomes strictly positive, anyone can callswap()
function to pull thetoken0
of that amount without any cost.So going back to the question, if the LP choose the values
amount0, amount1, liquidity
not accurately, the transaction reverts ora+b-c-d
becomes greater than zero.Generally, liquidity providers do not specify the desired liquidity amount in other protocols.
During the conversation with the sponsor team, it is understood that they avoided the calculation of
liquidity
fromamount0, amount1
because it is too complicated.Off-chain calculation will be necessary to help the situation, and this would limit the growth of the protocol.
If any other protocol is going to integrate Numoen, they will face the same problem.
I did some calculation and got the formula for the liquidity as below.
$$C=10^{18}$ , $x$ is $y$ is $P$ is the $L$ is the liquidity amount that should be used.
L = \frac{PCy+C^2x+\sqrt{2PC^3xy+C^4x^2}}{2P^2}
$
where
amount0
,amount1
,upperBound
,Because the LP will almost always suffer revert or fund loss without help of off-chain calculation, I submit this as a medium finding.
I would like to note that there still exists a mitigation (not that crazy).
As a side note, it would be very helpful to add new preview functions.
Tools Used
Manual Review
Recommended Mitigation Steps
Add a functionality to calculate the liquidity for the first deposit on-chain.
And it is also recommended to add preview functions.
The text was updated successfully, but these errors were encountered: