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

The yieldFeeRecipient will reduce the yieldFeeBalance to 0 everytime he withdraws less _shares than the maximum amount that he could. This can lead to loss of funds

Proof of Concept

Place the following into Liquidate.t.sol

  function testClaimYieldFeeShares_withdrawPartialBalance_checkYieldFeeBalance() 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 yieldFeeBalance = vault.yieldFeeBalance();
        assertGt(yieldFeeBalance, 0);
        
        vm.startPrank(bob);
        vm.expectEmit();
        emit Transfer(address(0), bob, yieldFeeBalance / 3);
        vm.expectEmit();
        emit ClaimYieldFeeShares(bob, yieldFeeBalance / 3);
        vault.claimYieldFeeShares(yieldFeeBalance / 3);
        vm.stopPrank();
        
        assertEq(vault.balanceOf(bob), yieldFeeBalance / 3);

        uint256 yieldFeeBalanceAfter = vault.yieldFeeBalance();
        console2.log("YieldFeeBalance after claiming yield fee shares: ",yieldFeeBalanceAfter);
        // YieldFeeBalance after claiming yield fee shares:  0
        assertEq(yieldFeeBalanceAfter, 0);
    }

Tools Used

Manual review

Recommended Mitigation Steps

Instead of subtracting yieldFeeBalance with _yieldFeeBalance you should subtract yieldFeeBalance with the _shares that the yieldFeeRecipient wants to withdraw

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

        uint256 _yieldFeeBalance = yieldFeeBalance;
        if (_shares > _yieldFeeBalance) revert SharesExceedsYieldFeeBalance(_shares, _yieldFeeBalance);
        //audit-high yieldFeeBalance decrease with _shares not make it 0
-       yieldFeeBalance -= _yieldFeeBalance;
+       yieldFeeBalance -= _shares;

        _mint(msg.sender, _shares);
    
        emit ClaimYieldFeeShares(msg.sender, _shares);
    }

Assessed type

Error