PoolTogether - sammy's results

Lines of code

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

Vulnerability details

The claimYieldFeeShares() function in PrizeVault.sol takes an input of uint256 _shares, which is the number of shares the yield fee recipient wishes to claim. However, while calculating the new value of the yieldFeeBalance, the function decrements it by _yieldFeeBalance (equal to yieldFeeBalance), effectively setting it to zero, irrespective of the value of _shares that the yield fee recipient inputs in the function call.

Impact

The yield fee recipient loses the remainder of their claimable balance (yieldFeeBalance - _shares) as yieldFeeBalance is set to zero.

Proof of Concept

The following Proof of Code has been written assuming a scenario wherein the yieldFeeRecipent claims half of the accrued yieldFeeBalance.

To run the test:

1. Copy the following code into the PrizeVault.t.sol file.

function testPoc() external{
        // set up the yield fee recipent and the yield fee percentage
        vault.setYieldFeePercentage(1e8); // 10%
        vault.setYieldFeeRecipient(bob);

        // make an initial deposit
        underlyingAsset.mint(alice, 1e18);
        vm.startPrank(alice);
        underlyingAsset.approve(address(vault), 1e18);
        vault.deposit(1e18, alice);
        vm.stopPrank();

        assertEq(vault.totalAssets(), 1e18);
        assertEq(vault.totalSupply(), 1e18);
        assertEq(vault.totalDebt(), 1e18);

        // mint yield to the vault and liquidate
        underlyingAsset.mint(address(vault), 1e18);
        vault.setLiquidationPair(address(this));
        uint256 maxLiquidation = vault.liquidatableBalanceOf(address(underlyingAsset));
        uint256 amountOut = maxLiquidation / 2;
        vault.transferTokensOut(address(0), bob, address(underlyingAsset), amountOut);

        uint256 yieldFeeBalance = vault.yieldFeeBalance();
        assertEq(vault.totalAssets(), 1e18 + 1e18 - amountOut); // existing balance + yield - amountOut
        assertEq(vault.totalSupply(), 1e18); // no change in supply since liquidation was for assets
        assertEq(vault.totalDebt(), 1e18 + yieldFeeBalance); // debt increased since we reserved shares for the yield fee
        
        vm.startPrank(bob);
        // the recipent claims half the yieldFeeBalance
        vault.claimYieldFeeShares(yieldFeeBalance/2);
        // however, the remaining yieldFeeBalance is not the expected amount
        assertNotEq(vault.yieldFeeBalance(), yieldFeeBalance/2);
        // the recipent loses their entire balance, as it is now 0
        assertEq(vault.yieldFeeBalance(), 0);
        vm.stopPrank();
    }

2. Run the test using forge test --match-test testPoc

Output :

Ran 1 test for test/unit/PrizeVault/PrizeVault.t.sol:PrizeVaultTest
[PASS] testPoc() (gas: 509919)
Suite result: ok. 1 passed; 0 failed; 0 skipped; finished in 3.16ms (754.40µs CPU time)

Ran 1 test suite in 115.40ms (3.16ms CPU time): 1 tests passed, 0 failed, 0 skipped (1 total tests)

Tools Used

Foundry

Recommended Mitigation Steps

Make the following change to the claimYieldFeeShares() function in PrizeVault.sol :

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

Assessed type

Error