PoolTogether - 0xJaeger's results

Lines of code

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

Vulnerability details

Impact

When yieldFeeRecipient calls the claimYieldFeeShares() function with an amount of shares that is less than the yieldFeeBalance, the yieldFeeBalance will be reset to 0. As a result, the difference between the claimed shares and the yieldFeeBalance will effectively be lost.

Proof of Concept

Here is a coded POC which can be directly pasted in Liquidate.t.sol

import {console2} from "forge-std/Test.sol";

function testClaimYieldFeeShares_withdrawPartial() public {
        vault.setYieldFeePercentage(1e8); // 10% fee
        vault.setYieldFeeRecipient(bob);
        vault.setLiquidationPair(address(this));

        // liquidate some yield
        underlyingAsset.mint(address(vault), 1e18);
        uint256 amountOut = vault.liquidatableBalanceOf(address(underlyingAsset));
        assertGt(amountOut, 0);

        vault.transferTokensOut(address(0), alice, address(underlyingAsset), amountOut);
        uint256 yieldFeeBalanceBefore = vault.yieldFeeBalance();
        console2.log("YieldFeeBalance before claim: ", yieldFeeBalanceBefore);
        // YieldFeeBalance before claim: 99999999999900000

        vm.prank(bob);
        vault.claimYieldFeeShares(yieldFeeBalanceBefore / 3);
        console2.log("Balance of Bob after claim: ", vault.balanceOf(bob));
        // Balance of Bob after claim: 33333333333300000

        uint256 yieldFeeBalanceAfter = vault.yieldFeeBalance();
        console2.log("YieldFeeBalance after claim: ", yieldFeeBalanceAfter);
        // YieldFeeBalance after claim: 0
        // The yieldFeeBalanceAfter is 0 after the claim, but should be 66666666666600000 (yieldFeeBalanceBefore - yieldFeeBalanceBefore / 3)
    }

Tools Used

Manual Analysis

Recommended Mitigation Steps

The error is caused from a logical error inside the claimYieldFeeShares() here is the suggested patch to fix this error in PrizeVault.sol:

 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