Lack of zero value checks for minShares slippage value can lead to loss of shares for users

[c4-bot-9]

Lines of code

https://github.com/code-423n4/2024-02-spectra/blob/383202d0b84985122fe1ba53cfbbb68f18ba3986/src/tokens/PrincipalToken.sol#L192 https://github.com/code-423n4/2024-02-spectra/blob/383202d0b84985122fe1ba53cfbbb68f18ba3986/src/tokens/PrincipalToken.sol#L220

Vulnerability details

Impact

Users can submit a minShares value of zero by error or not understanding impact this implies there is no slippage protection and the user can get back zero shares for the vault as this will still be considered acceptable.

Proof of Concept

https://github.com/code-423n4/2024-02-spectra/blob/383202d0b84985122fe1ba53cfbbb68f18ba3986/src/tokens/PrincipalToken.sol#L192

function deposit(
        uint256 assets,
        address ptReceiver,
        address ytReceiver,
        uint256 minShares
    ) external override returns (uint256 shares) {
        shares = deposit(assets, ptReceiver, ytReceiver);
        if (shares < minShares) {
            revert ERC5143SlippageProtectionFailed();
        }
    }

If minShares is passed in as zero above the depositor can get back zero shares as function wont revert resulting in value loss. Users should be able to set minShares that must never be allowed to be 0

Tools Used

Manual analysis

Recommended Mitigation Steps

Ensure there is a check that minShares != 0 Can maybe add other checks based on previews,offchain or other means to ensure minShares is within reasonable bounds

Assessed type

Other

[c4-pre-sort]

gzeon-c4 marked the issue as duplicate of #246

[c4-pre-sort]

gzeon-c4 marked the issue as insufficient quality report

[gzeon-c4]

user error

[c4-judge]

JustDravee marked the issue as unsatisfactory: Invalid