PoolTogether - DanielTan_MetaTrust's results

Lines of code

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

Vulnerability details

Impact

The fee recipient probably loses the yield fee after the first claim if the _shares are less than the _yieldFeeBalance.

In the claimYieldFeeShares function, the yieldFeeBalance will be set to be zero:

    function claimYieldFeeShares(uint256 _shares) external onlyYieldFeeRecipient {
        ...

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

        yieldFeeBalance -= _yieldFeeBalance; 

        ...
    }

Thus, when the parameter _shares is less than the yieldFeeBalance, the yieldFeeBalance is updated to be zero un-expectedly, which results in the yield fee loss for the fee recipient.

Proof of Concept

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

When the recipient invokes the claimYieldFeeShares function, assume the yieldFeeBalance is 100, and the parameter _shares is 1.

• the _yieldFeeBalance becomes 100;
• the yieldFeeBalance updated to be zero, because 100 -= 100 results in the yieldFeeBalance becomes zero;
• mint 1 token to the recipient;
• emit an event that the recipient claimed 1 share.

The real share minted to the recipient is 1 share, but the yieldFeeBalance is set to 0 unexpectedly. If the recipient wants to claim the rest 99 shares, the function will revert because the condition _shares > _yieldFeeBalance is true.

Tools Used

Manual Review

Recommended Mitigation Steps

Recommend updating the calculation of yieldFeeBalance as below:

yieldFeeBalance -= _shares;

Assessed type

Context