Funds could stuck in `Pools.sol` contract

[c4-bot-2]

Lines of code

https://github.com/code-423n4/2024-01-salty/blob/main/src/pools/Pools.sol#L219

Vulnerability details

Impact

Approximately, the dust amount could become stuck in the contract for every depositor

Proof of Concept

https://github.com/code-423n4/2024-01-salty/blob/main/src/pools/Pools.sol#L219

The withdraw() function is used for withdrawing the amount of the deposited token, requiring amount > PoolUtils.DUST to be true. This means if a user's balance falls below PoolUtils.DUST, they cannot withdraw all their deposits.

In contrast, other functions like removeLiquidity() allow users to remove all their deposits from a pool.

Tools Used

Manual

Recommended Mitigation Steps

Add a check to ensure the caller doesn't end up in a position where _userDeposits[msg.sender][token] < PoolUtils.DUST, preventing the caller from being unable to exit completely.

Assessed type

Other

[c4-judge]

Picodes marked the issue as unsatisfactory: Overinflated severity

[Tri-pathi]

Hey @Picodes Thanks for smooth judging

The above issue describes flaws in withdrawals, since the minimum collateral isn't enforced, so users can reach a position below the dust threshold, which cannot be withdrawn further. The root cause of the above issue is similar to #279 and #784 (784 also shows that reserve1 can be less than dust; the same goes here for above explained issue)

I have explained enough to make the issue clear, but if it's not, here's an example:

Alice deposits token A for which amount > PoolUtils.DUST

She withdraw token A for which amount is _userDeposits[msg.sender][tokenA]-(PoolUtils.DUST-1)

Now, PoolUtils.DUST-1 amount will be stuck, as as withdrawals won't allow transferring tokens which are < PoolUtils.DUST

It seems to be a valid concern, similar to 784.

[Picodes]

784 highlights a missing check in the code and a bug. Here this report shows that up to dust can be lost by a given user, which is:

• not important as dust is a very small amount as its name suggests. So there is no real impact here
• quite obvious when you check the code

[Tri-pathi]

@Picodes I agree it's even dust but it can be prevented.

doesn't qualify for valid Low/QA?