PoolTogether - Greed'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

Yield that was supposed to be granted to users (or could have been granted to the feeRecipient) can't be claimed anymore.

Proof of concept

When the yieldVault earns yield, the feeRecipient can call claimYieldFeeShares() to earn shares of the PrizeVault.

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

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

The claimable shares can't exceed the yieldFeeBalance, which is increased in the transferTokensOut() function.

function transferTokensOut(
    address,
    address _receiver,
    address _tokenOut,
    uint256 _amountOut
) public virtual onlyLiquidationPair returns (bytes memory) {
    // ...
    // Increase yield fee balance:
    if (_yieldFee > 0) {
        yieldFeeBalance += _yieldFee;
    }
    // ...
}

The issue occurs when claimYieldFeeShares() is called with a parameter that is less than the yieldFeeBalance.

In that case, the shares of all the users won't match the amount of assets stored in the yieldVault which will result in some assets to be stuck in, with no way to claim them.

The following test can be added to Liquidate.t.sol and is based upon testClaimYieldFeeShares_withdrawFullBalance.

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

    // Alice deposits assets
    deal(address(underlyingAsset), alice, 1e18);
    console.log("Alice assets before        : ", underlyingAsset.balanceOf(alice));
    vm.startPrank(alice);
    console.log("[*] Alice deposits...");
    underlyingAsset.approve(address(vault), 1e18);
    vault.deposit(underlyingAsset.balanceOf(alice), alice);
    console.log("Alice shares obtained      : ", vault.balanceOf(alice));
    console.log("Alice assets after deposit : ", underlyingAsset.balanceOf(alice));
    vm.stopPrank();

    // 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 yieldFeeBalance = vault.yieldFeeBalance();
    assertGt(yieldFeeBalance, 0);
    
    vm.startPrank(bob);
    console.log("[*] Bob claims only 1 yield fee shares");
    vault.claimYieldFeeShares(1);
    vm.stopPrank();

    // Since bob has some shares, he can withdraw some assets
    vm.startPrank(bob);
    console.log("[*] Bob obtained shares and now withdraws some assets");
    vault.withdraw(1, bob, bob);
    console.log("Bob assets obtained : ", underlyingAsset.balanceOf(bob));
    vm.stopPrank();

    // Alice withdraws her assets, making a profit but there is yield stuck in yield vault
    vm.startPrank(alice);
    console.log("[*] Alice withdraws her initial deposit");
    vault.withdraw(1e18, alice, alice);
    console.log("Alice assets after  : ", underlyingAsset.balanceOf(alice));
    vm.stopPrank();

    console.log("Assets stuck in yield vault :", underlyingAsset.balanceOf(address(vault.yieldVault())));
    console.log("Shares of Bob               :", vault.balanceOf(bob));
    console.log("Shares of Alice             :", vault.balanceOf(alice));
}

Tools used

Manual analysis

Foundry tests

Recommended mitigation steps

Automatically mint the corresponding shares to the fee recipient in the claimYieldFeeShares()

Assessed type

Token-Transfer