Olas - adeolu's results

Lines of code

https://github.com/code-423n4/2023-12-autonolas/blob/2a095eb1f8359be349d23af67089795fb0be4ed1/lockbox-solana/solidity/liquidity_lockbox.sol#L277

Vulnerability details

Impact

in liquidity_lockbox.sol, users can withdraws LP tokens to get the underlying tokens via withdraw(). The withdraw() makes use of whirlpool.decreaseLiquidity() to convert a LP tokens amount to the underlying tokens. In the call to whirlpool.decreaseLiquidity(), the tokenMinA and tokenMinB parameters are hardcoded to be 0. Meaning the call will not revert if zero amount of token A and token B is gotten from the call. This will mean that users can lose money in some special cases and unknowingly get zero value for their LP tokens. There is not enough slippage protection for users.

Orca whirlpool is an AMM on solana. Any removal of liquidity from the AMM pool might result in slippage whereby the final amount withdrawn differs from the expected amount.

It is better to allow for users to set or determine the minimum amount of tokens they expect to get from a trade, swap or whenever they are exiting a liquidity position.

Proof of Concept

https://github.com/code-423n4/2023-12-autonolas/blob/2a095eb1f8359be349d23af67089795fb0be4ed1/lockbox-solana/solidity/interfaces/whirlpool.sol#L193

	function decreaseLiquidity(uint128 liquidityAmount,uint64 tokenMinA,uint64 tokenMinB) external;

https://github.com/code-423n4/2023-12-autonolas/blob/2a095eb1f8359be349d23af67089795fb0be4ed1/lockbox-solana/solidity/liquidity_lockbox.sol#L277

        whirlpool.decreaseLiquidity{accounts: metasDecreaseLiquidity, seeds: [[pdaProgramSeed, pdaBump]]}(amount, 0, 0);

in this code snippet above, we can see that in line 277 of liquidity_lockbox.sol, the tokenMinA and tokenMinB are actually hardcoded to be 0 at all times. This doesnt take into consideration some extreme conditions where large slippage can occur and it puts the user at risk of loss since user has no control over the output. User may expect to get token amounts more than 0 but can actually get nothing.

Tools Used

manual review.

Recommended Mitigation Steps

add extra parameters that allow the users to set the tokenMinA and tokenMinB amounts to be used in the whirlpool call. for example

    function withdraw(uint64 amount, uint64 tokenMinA, uint64 tokenMinB) external {

and in line 277

        whirlpool.decreaseLiquidity{accounts: metasDecreaseLiquidity, seeds: [[pdaProgramSeed, pdaBump]]}(amount, tokenMinA, tokenMinB);

Assessed type

Other