PoolTogether - btk's results

Lines of code

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

Vulnerability details

Overview

The PrizeVault facilitates asset deposits and accrues yield through an associated yield vault. This accrued yield is anticipated to be liquidated and allocated to the prize pool as prize tokens.

However, during the liquidation process, a fee is imposed, which accumulates in the yieldFeeBalance. The intended recipient of this fee can either claim the entire balance or opt to claim only a portion of it.

The issue arises when the feeRecipient opts to claim a partial amount, resulting in the forfeiture of the remaining balance.

Impact

Permanent loss of fee funds when the feeRecipient chooses to claim only a portion of the yieldFeeBalance.

Proof of Concept

The function claimYieldFeeShares is utilized by the feeRecipient to claim their portion of the yieldFeeBalance::

    function claimYieldFeeShares(uint256 _shares) external onlyYieldFeeRecipient {

        uint256 _yieldFeeBalance = yieldFeeBalance;

        yieldFeeBalance -= _yieldFeeBalance;
        
        _mint(msg.sender, _shares);

    }

In this implementation, the _yieldFeeBalance is mistakenly subtracted from the yieldFeeBalance storage, resulting in the deletion of the entire balance instead of deducting the claimed shares.

For instance, if yieldFeeBalance is 1000 and the feeRecipient claims only 100 shares, they should be left with 900 shares. However, due to the flawed implementation, the entire balance is deleted, leaving the feeRecipient with 0 shares.

Tools Used

Manual review

Recommended Mitigation Steps

Consider subtracting the claimed shares from the yieldFeeBalance, :

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

        uint256 _yieldFeeBalance = yieldFeeBalance;
        if (_shares > _yieldFeeBalance) revert SharesExceedsYieldFeeBalance(_shares, _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

Vulnerability details

Overview

The withdraw function in the PrizeVault lacks a mechanism for users to express their minimum acceptable output. This deficiency exposes users to potential losses of their principal due to yield strategy losses.

Impact

Users will loss assets due to the absence of slippage control in the withdrawal function.

Proof of Concept

• In the PrizeVault depositors should always expect to be able to withdraw their full deposit amount and no more as long as global withdrawal limits meet or exceed their balance.
• However, if the underlying yield source loses assets, depositors will only be able to withdraw a proportional amount of remaining assets based on their share balance and the total debt balance.
• The current implementation of the PrizeVault's withdraw function lacks protection, with no provisions for a minimum return amount or a deadline for valid trade transactions. This leaves transactions vulnerable to delays on congested networks.

    function withdraw(
        uint256 _assets,
        address _receiver,
        address _owner
    ) external returns (uint256) {
        uint256 _shares = previewWithdraw(_assets);
        _burnAndWithdraw(msg.sender, _receiver, _owner, _shares, _assets);
        return _shares;
    }

• For instance, if a user attempts to withdraw assets from the vault and their transaction is delayed due to low gas or network congestion, any loss in the yield strategy's assets would result in the trade occurring at a sub-optimal price, harming the user.

Tools Used

Manual review

Recommended Mitigation Steps

Introduce a router for users since adding a min minOut input would not be "standard".

Assessed type

Other