pashap9990 - Lp amount will be minted lower than what it really expected in mint function

[sherlock-admin3]

pashap9990

Medium

Lp amount will be minted lower than what it really expected in mint function

Summary

LPs may receive fewer tokens than expected when minting due to fluctuating pool reserves. Without specifying a minimum LP amount, this can lead to unexpected fund losses for LPs. Adding a min_lp_amount parameter to the Api::mint function can prevent this issue

Internal pre-conditions

Consider change config in tests/conftest.py
I do it for better understanding,Fee doesn't important in this issue

PARAMS = {
  'MIN_FEE'               : 0,
  'MAX_FEE'               : 0,

  'PROTOCOL_FEE'          :1000
    ...
}

Code Snippet

https://github.com/sherlock-audit/2024-08-velar-artha/blob/main/gl-sherlock/contracts/api.vy#L75

Impact

Lp amount will be minted lower than what it really expected

PoC

Textual PoC:
we assume protocol fee is zero in this example
1-Alice opens long position when price is $2
2-Joe opens a short position when price is $2
3-Price goes down to $1
4-Bob calls Pools::calc_mint and he realize if he provide 1000 VEL to pool,he gets 1000 LP
5-Bob send his tx to network
6-liquidator bot liquids Alice's position
7-Bob's tx will be executed and Bob get 952 lp instead of 1000
8-Joe closes his position and gets his profit,hence pool's reserve will be deducted
9-Bob burns his LPs token,and he gets 772 VEL instead of 1000 VEL
Coded PoC:
Place below test in tests/test_positions.py and run pytest -k test_get_less_lp_amt_than_expected - s

def test_get_less_lp_amt_than_expected(setup, VEL, STX, lp_provider, LP, pools, open, long, close, burn, liquidator, liquidate, mint, short):
    setup()
    #Alice opens position
    open(VEL, STX, True, d(1000), 5, price=d(2), sender=long)
    #Joe opens short position
    open(VEL, STX, False, d(1000), 5, price=d(2), sender=short)

    #Bob calls calc_mint 
    assert pools.calc_mint(1, d(1000), 0, d(20_000), ctx(d(1))) == d(1000)

    #Alice will be liquidated
    liquidate(VEL, STX, 1, price = d(1), sender = liquidator)

    #Bob's tx will be executed
    mint(VEL, STX, LP, d(1000), 0, price=d(1), sender=lp_provider)    

    assert LP.totalSupply() == 20952426306#its mean,Bob get 48 lp token lower than expected

    #Joe closes his position
    close(VEL, STX, 2, price=d(1), sender=short)


    vel_bef = VEL.balanceOf(lp_provider)
    #Bob burn his lp tokens
    burn(VEL, STX, LP, 952426306, price=d(1), sender=lp_provider)
    
    vel_after = VEL.balanceOf(lp_provider)

    print("vel_diff:", (vel_after - vel_bef) / 1e6)#Bob lost $228 

Mitigation

Api::mint should have another parameter like min_lp_amount which can be checked when the lp token wants to be mint

Duplicate of #74

[rickkk137]

Escalate
LP token price directly compute based pool reserve and total supply lp token and the issue clearly states received amount can be less than expected amount and in Coded PoC liquidity provider expected $1000 but in result get $772

loss = $228[22%]

Causes a loss of funds but requires certain external conditions or specific states, or a loss is highly constrained. The loss of the affected party must exceed 0.01% and 10 USD

[sherlock-admin3]

Escalate

Slippage related issues showing a definite loss of funds with a detailed explanation for the same can be considered valid high

You've created a valid escalation!

To remove the escalation from consideration: Delete your comment.

You may delete or edit your escalation comment anytime before the 48-hour escalation window closes. After that, the escalation becomes final.

[WangSecurity]

After additionally considering this issue, indeed, the slippage is checked only on the oracle price and doesn't catch the pool reserves, leading to the situation where regardless of user-set slippage, they can get fewer tokens after mint. This issue exists in both mint and burn functions. The root cause is the same - insufficient slippage protection and the fix would be the same, even though they should be fixed separately. Hence, this issue will be duplicated with #74, planning to accept the escalation.

[WangSecurity]

Result:
Medium
Duplicate of #74

[sherlock-admin4]

Escalations have been resolved successfully!

Escalation status:

• rickkk137: accepted