The entire yieldFeeBalance could be permanently lost due to wrong balance update

[c4-bot-5]

Lines of code

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

Vulnerability details

Impact

The entire yieldFeeBalance could be permanently lost due to wrong balance update

Proof Of Concept

The issue is in the logic of the claimYieldFeeShares() function which is intended to allow the yield fee recipient to transfer out yield fee shares. The caller is allowed to specify a non-zero amount of the fee shares to claim and checks that the amount specified is not more than the fee balance else reverts. However, after this check if updates the yield fee balance and then mint the shares for the recipient. The issue here is that the yield fee balance gets updated by the overall balance instead of the amount being claimed:

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

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

@>      yieldFeeBalance -= _yieldFeeBalance;

        _mint(msg.sender, _shares);

        emit ClaimYieldFeeShares(msg.sender, _shares);
    }

As you can see, it minuses the entire balance regardless of the amount of shares to be claimed. This means even when a fee recipient attempts to claim any small amount of fee share, the entire yieldFeeBalance will be permanently lost.

Tools Used

Manual Review

Recommendation

I recommend adding this fix:

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

Assessed type

Error

[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 marked the issue as satisfactory