claimYieldFeeShares rests yieldFeeBalance every time even if not all fees are claimed

[c4-bot-6]

Lines of code

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

Vulnerability details

Impact

PrizeVault::claimYieldFeeShares can only be called by the yieldFeeRecipient to claim the fees. The yieldFeeRecipient can specify the exact amount of shares he wants to claim, however this is a problem because the yieldFeeBalance is reset every time, no matter how many shares the yieldFeeRecipient claimed.

Proof of Concept

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

    uint256 _yieldFeeBalance = yieldFeeBalance;
    if (_shares > _yieldFeeBalance) revert SharesExceedsYieldFeeBalance(_shares, _yieldFeeBalance);
    // @audit this should be shares
    yieldFeeBalance -= _yieldFeeBalance;

    _mint(msg.sender, _shares);

    emit ClaimYieldFeeShares(msg.sender, _shares);
}

We can see that first the yieldFeeBalance is stored in _yieldFeeBalance and then subtracted with the same amount even if the yieldFeeRecipient doesn't claim all the fees.

This leaves unclaimed fees and affects the _totalDebt since it would return less than it actually is.

Tools Used

Manual Review

Recommended Mitigation Steps

Subtract the shares from yieldFeeBalance instead of reseting it.

    yieldFeeBalance -= _shares;

Assessed type

Other

[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