pashap9990 - LPs cannot specify min amount received in burn function, causing loss of fund for them

[sherlock-admin3]

pashap9990

Medium

LPs cannot specify min amount received in burn function, causing loss of fund for them

Summary

LPs cannot set minimum base or quote amounts when burning LP tokens, leading to potential losses due to price fluctuations during transactions.

Root Cause

LPs cannot set the base received amount and the quote received amount

Impact

LPs may receive significantly lower amounts than expected when burning LP tokens, resulting in financial losses

Code Snippet

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

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
    ...
}

PoC

Textual PoC:
we assume protocol fee is zero in this example
1-Bob mints 20,000e6 LP token[base_reserve:10,000e6, quote_reserve:10,000e6]
2-Alice opens long position[collateral 1000 STX, LEV:5,Price:1]
3-Price goes up til $2
4-Bob calls calc_burn[lp_amt:10,000e6,total_supply:20,000e6][return value:base 3750 VEL,quote 7500 STX]
5-Bob calls burn with above parameters
6-Alice calls close position
7-Alice's tx executed before Bob's tx
8-Bob's tx will be executed and Bob gets 3875 VEL and 4750 STX
9-Bob losts $2500
Coded PoC:
place this test in tests/test_positions.py and run this command pytest -k test_lost_assets -s

def test_lost_assets(setup, VEL, STX, lp_provider, LP, pools, math, open, long, close, burn):
    setup()
    #Alice opens position
    open(VEL, STX, True, d(1000), 5, price=d(1), sender=long)
    
    reserve = pools.total_reserves(1)
    assert reserve.base == 10000e6
    assert reserve.quote == 10000e6
    #Bob calls calc_burn, Bob's assets in term of dollar is $15,000
    amts = pools.calc_burn(1, d(10000)  , d(20000), ctx(d(2)))
    
    assert amts.base == 3752500000
    assert amts.quote == 7495000000

    #Alice closes her position
    bef = VEL.balanceOf(long)
    close(VEL, STX, 1, price=d(2), sender=long)
    after = VEL.balanceOf(long)

    vel_bef = VEL.balanceOf(lp_provider)
    stx_bef = STX.balanceOf(lp_provider)

    amts = pools.calc_burn(1, d(10000)  , d(20000), ctx(d(2)))
    assert amts.base == 3877375030
    assert amts.quote == 4747749964
    #Bob's tx will be executed
    burn(VEL, STX, LP, d(10000), price=d(2), sender=lp_provider)

    vel_after = VEL.balanceOf(lp_provider)
    stx_after = STX.balanceOf(lp_provider)


    print("vel_diff:", (vel_after - vel_bef) / 1e6)#3877.37503 VEL
    print("stx_diff:", (stx_after - stx_bef) / 1e6)#4747.749964 STX
    #received values in term of dollar is ~ $12,500,Bob lost ~ $2500

Mitigation

Consider adding min_base_amount and min_quote_amount to the burn function's params or adding min_assets_value for example when the price is $2 LPs set this param to $14800, its mean received value worse has to be greater than $14800

[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 $15000 but in result get $12500

loss = $2500[1.6%]

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]

However, the LPs can add input slippage parameters, i.e. desired and slippage to mitigate these issues. Am I missing something here?

[rickkk137]

@WangSecurity desired and slippage just has been used to control price which fetch from oracle and protocol uses of that for converting quote to base or base to quote to compute total pool's reserve in terms of quote token but there is 2 tips here

• burn value = lp_amt * pool_reserve / total_supply_lp
• pool_reserve will be decreased when users closes their profitable positions

let's examine an example with together:
u have 1000 lp token and u want convert them to usd and pool_reserve and total_supply_lp is 1000 in our example,
burn_value = lp_amt * pool_reserve / total_supply = 1000 * 1000 / 1000 = 1000 usd
based on above value u send your transaction to network but a close profitable transaction will be executed before your transaction and get $100 as payout,its mean pool reserve is 900
burn_value = lp_amt * pool_reserve / total_supply = 1000 * 900 / 1000 = 900 usd
u get $900 instead of $1000 and u cannot control this situation as a user

[WangSecurity]

Yeah, I see, thank you. Indeed, the situation which would cause an issue to the user happens after the slippage is checked and this conversion cannot be controlled by the user. Planning to accept the escalation and validate with medium severity. Are there any duplicates (non-escalated; I see there are some escalations about slippage; I will consider them as duplicates if needed)?

[WangSecurity]

Result:
Medium
Has duplicates

[sherlock-admin4]

Escalations have been resolved successfully!

Escalation status:

• rickkk137: accepted