The `claimYieldFeeShares` function will lose shares if `shares` != `yieldFeeBalance`

[c4-bot-7]

Lines of code

https://github.com/code-423n4/2024-03-pooltogether/blob/480d58b9e8611c13587f28811864aea138a0021a/pt-v5-vault/src/PrizeVault.sol#L611

Vulnerability details

[M-1] The claimYieldFeeShares function will lose shares if shares != yieldFeeBalance

Description There is a mishandling of yieldFeeBalance in the claimYieldFeeShares function. this function will set the yieldFeeBalance to zero every time is called and doesn't handle the shares well:

    function claimYieldFeeShares(uint256 _shares) external onlyYieldFeeRecipient {
        if (_shares == 0) revert MintZeroShares();

        uint256 _yieldFeeBalance = yieldFeeBalance;
        if (_shares > _yieldFeeBalance) revert SharesExceedsYieldFeeBalance(_shares, _yieldFeeBalance);

        // @audit-medium the `YieldFeeRecipient` lose shares if shares != yieldFeeBalance
        // it should be `yieldFeeBalance -= _shares;` instead for mitigation
        yieldFeeBalance -= _yieldFeeBalance;

        _mint(msg.sender, _shares);

        emit ClaimYieldFeeShares(msg.sender, _shares);
    }

the claimYieldFeeShares function must subtract yieldFeeBalance with shares not with itself because for example if yieldFeeBalance was 10e18 and the yieldFeeRecipient wants just to call the claimYieldFeeShares with 1e18 shares then he would lose the other 9e18 shares which could mint. he had this potential to mint shares more and just lost it.

Impact funds are not at direct risk but yieldFeeRecipient will just lose shares that he could minted.

Recommend Mitigation

-   yieldFeeBalance -= _yieldFeeBalance;
+   yieldFeeBalance -= _shares;

Assessed type

Math

[c4-pre-sort]

raymondfam marked the issue as sufficient quality report

[c4-pre-sort]

raymondfam marked the issue as duplicate of #10

[c4-pre-sort]

raymondfam marked the issue as duplicate of #59

[c4-judge]

hansfriese changed the severity to 3 (High Risk)

[c4-judge]

hansfriese marked the issue as satisfactory