PoolTogether - FastChecker's results

Lines of code

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

Vulnerability details

Impact

The recipient of the yieldfee might suffer a loss of the yield fee.

Proof of Concept

The PrizeVault.sol#claimYieldFeeBalance() function is as follows.

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

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

617     yieldFeeBalance -= _yieldFeeBalance;

        _mint(msg.sender, _shares);

        emit ClaimYieldFeeShares(msg.sender, _shares);
    }

As observed in the code snippet on the right, in L617, yieldFeeBalance is initialized to 0 irrespective of the amount of shares to be minted to the yield fee recipient. Therefore, the recipient of the yield fee loses the fee when _shares < _yieldFeeBalance.

Tools Used

Manual Review

Recommended Mitigation Steps

Modify the PrizeVault.sol/claimYieldFeeBalance() function as follows.

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

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

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

        _mint(msg.sender, _shares);

        emit ClaimYieldFeeShares(msg.sender, _shares);
    }

Assessed type

Other

Lines of code

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

Vulnerability details

Impact

Loss of assets for the affected users.

Proof of Concept

The PrizeVault.sol#withdraw() and PrizeVault.sol#redeem() functions call the PrizeVault.sol#previewWithdraw() and PrizeVault.sol#previewRedeem() functions to calculate the amount of stocks to be burned or assets to be redeemed. When users deposit into PrizeVault they are really depositing into an ERC4626 vault that contributes its yield to the Prize Pool. Therefore, when executing a transaction, the entire assets of PrizeVault can rise or fall at any time depending on the current onchain status according to the stock value of the yield vault.

    function totalAssets() public view returns (uint256) {
        return yieldVault.convertToAssets(yieldVault.balanceOf(address(this))) + _asset.balanceOf(address(this));
    }

Therefore, if PrizeVault's total assets become less than totalDebt, slippage may occur in these functions(previewWithdraw(), previewRedeem()). This slippage is more than what they can accept. In summary, the PrizeVault.sol#withdraw(), PrizeVault.sol#redeem() functions lack slippage control that allows the user to revert if the amount of shares the user redeems is greater than expected or if the amount of assets the user receives is less than expected.

Tools Used

Manual Review

Recommended Mitigation Steps

Implement slip control in PrizeVault.sol#withdraw(), PrizeVault.sol#redeem() functions.

Assessed type

Other