PoolTogether - gesha17's results

Lines of code

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

Vulnerability details

Impact

A yield fee recipient calls the claimYieldFeeShares() function to claim yield liquidation fees from the PrizeVault. The recipient specifies as input the number of shares he wants to claim, however regardless of the amount specified the function always resets the yield fee balance to 0. So, unless the yield fee recipient always calls the function with the exact amount he will lose fees.

Proof of Concept

Yield fees are accumulated on token liquidations in the prize vault. Each liquidation contributes a percentage to the yieldFeeBalance.

When enough fees are accumulated, the yield fee recipient can withdraw them. He does so through the claimYieldFeeShares() function:

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

    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 we can see the receiver speicifies a _shares input parameter for the amount of shares he wants to receive. These shares are minted to the msg.sender. However regardless of this amount the yieldFeeBalance is reset to 0:

        yieldFeeBalance -= _yieldFeeBalance;

This means that input parameter _shares must always match yieldFeeBalance to prevent loss of funds. Even if the recipient tries to match variable, liquidation transactions that frontrun the claimYieldFeeShares() transaction will cause loss of funds.

Tools Used

Manual Review

Recommended Mitigation Steps

The correct calucation should be:

        yieldFeeBalance -= _shares;

This will accurately reflect the shares minted to the user.

Assessed type

Math