There is no check against setting "minOut" to zero

[hats-bug-reporter]

Github username: --
Twitter username: 97Sabit
Submission hash (on-chain): 0x3d7281573f7522cc131a8e44606498a2f7738e86c5a0d81018b020878d40b42e
Severity: medium

Description:
Description
Users are at liberty to set minOut to any amount they want in the localSwap function.

But the rationale behind minOut is to serve as a protection against getting zero amount from a swap and frontrunning attack.

However, there is no check in place to ensure that minOut cannot be set to zero. Users can knowingly or mistakenly set minOut to zero. This exposes users to frontrunning attack and getting zero amount from a swap.

1. Proof of Concept (PoC) File

https://github.com/catalystdao/catalyst/blob/27b4d0a2bca177aff00def8cd745623bfbf7cb6b/evm/src/CatalystVaultVolatile.sol#L565-L572

[reednaa]

Some users might want to set minout to zero.

If we didn't accept zero as a valid minout, they could easily circumvent the minout by setting it to 1. It is true that the explicit way is more verbose.

The latter recommendation is a dublicate of #13

[ololade97]

Another weakness to mention is that a lot of users tend to not set minOut values, and swapping withing a vault via localSwap() and sending asset via sendAsset() and other vault actions does not check that the minOut value is actually zero.

The above is the relevant excerpt from issue #13. However, it's also not similar to this issue.

It's true some users may want to set minOut to zero. But they are at the risk of frontrunning.