PoolTogether - AcT3R's results

Lines of code

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

Vulnerability details

Impact

The claimYieldFeeShares method is intended to claim the yield fee in shares based on _shares parameter. The yield fee recipient passes the number of shares it wants to claim, which is then compared with the yieldFeeBalance which should represent the amount of claimable shares for the recipient. In case _shares parameter is lower than yieldFeeBalance, it will mint the _shares amount of shares but will reset the yieldFeeBalance to 0 in any case. The result is that the yield fee recipient receives fewer shares than it should, and the value that would come from the rest of the shares is lost to him and the assets are shared with the depositors.

Proof of Concept

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);
    }

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

Tools Used

Manual review

Recommended Mitigation Steps

There are multiple possible solutions for the issue.

One is to remove _shares parameter and mint the number of shares to the fee recipient based on the yieldFeeBalance value.

   function claimYieldFeeShares() external onlyYieldFeeRecipient {
        uint256 _yieldFeeBalance = yieldFeeBalance;

        yieldFeeBalance = 0;

        _mint(msg.sender, _yieldFeeBalance);

        emit ClaimYieldFeeShares(msg.sender, _yieldFeeBalance);
   }

The other one is to leave _shares parameter but properly calculate the yieldFeeBalance based on the _shares parameter.

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

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

        yieldFeeBalance -= _shares;

        _mint(msg.sender, _shares);

        emit ClaimYieldFeeShares(msg.sender, _shares);
    }

Assessed type

Error