PoolTogether - 0xhunter20's results

Lines of code

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

Vulnerability details

Impact

yieldFeeBalance wouldn't be claimed after calling transferTokensOut() due to the twab supply limit.

Proof of Concept

When _tokenOut == address(this), liquidatableBalanceOf() mints shares to the receiver and accumulates yieldFeeBalance accordingly.

But when it checks the maximum liquidatable amount in liquidatableBalanceOf(), it validates the twap supply limit with the liquidYield only and it might meet the supply limit while minting yieldFeeBalance like the below.

• We assume totalSupply = 6e28, yieldFeeBalance = 0, twabSupplyLimit = 2^96 - 1 = 7.9e28 and the vault has enough available yield.
• Then liquidatableBalanceOf(_tokenOut = address(this)) will return _maxAmountOut = 7.9e28 - 6e28 = 1.9e28 when _liquidYield > _maxAmountOut.
• After calling transferTokensOut() with _amountOut = 1.9e28, _yieldFee will be added to yieldFeeBalance but it can't be claimed as we met the twap supply limit already.

Tools Used

Manual Review

Recommended Mitigation Steps

liquidatableBalanceOf() shouldn't apply yieldFeePercentage to compare with _maxAmountOut when _tokenOut == address(this).

    function liquidatableBalanceOf(address _tokenOut) public view returns (uint256) {
        uint256 _totalSupply = totalSupply();
        uint256 _maxAmountOut;
        if (_tokenOut == address(this)) {
            // Liquidation of vault shares is capped to the TWAB supply limit.
            _maxAmountOut = _twabSupplyLimit(_totalSupply);
        } else if (_tokenOut == address(_asset)) {
            // Liquidation of yield assets is capped at the max yield vault withdraw plus any latent balance.
            _maxAmountOut = _maxYieldVaultWithdraw() + _asset.balanceOf(address(this));
        } else {
            return 0;
        }

        // The liquid yield is computed by taking the available yield balance and multiplying it
        // by (1 - yieldFeePercentage), rounding down, to ensure that enough yield is left for the
        // yield fee.
        uint256 _liquidYield = _availableYieldBalance(totalAssets(), _totalDebt(_totalSupply));

        if (_tokenOut == address(this)) {
            if (_liquidYield >= _maxAmountOut) { //compare before applying yieldFeePercentage 
                _liquidYield = _maxAmountOut;
            }
            _liquidYield = _liquidYield.mulDiv(FEE_PRECISION - yieldFeePercentage, FEE_PRECISION);
        } else {
            _liquidYield = _liquidYield.mulDiv(FEE_PRECISION - yieldFeePercentage, FEE_PRECISION);

            if (_liquidYield >= _maxAmountOut) { //same as before
                _liquidYield = _maxAmountOut;
            }
        }

        return _liquidYield;
    }

Assessed type

Invalid Validation

Lines of code

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

Vulnerability details

Impact

The yield fee recipient couldn't claim his yield fee shares due to the twap supply limit.

Proof of Concept

The yield fee recipient can claim already earned yield fee shares using claimYieldFeeShares().

Due to the TwabController's limits, we check the available supply using _twabSupplyLimit() but we don't check yieldFeeBalance that can be claimed any time.

So the below case would be possible.

• We assume totalSupply = 6e28, yieldFeeBalance = 1e28, twabSupplyLimit = 2^96 - 1 = 7.9e28.
• maxDeposit() will return twabSupplyLimit - totalSupply = 1.9e28.
• A user calls deposit() with the above maximum amounts and totalSupply = twabSupplyLimit now.
• Then claimYieldFeeShares() will revert due to the twap supply limit.

Tools Used

Manual Review

Recommended Mitigation Steps

_twabSupplyLimit() should calculate the remaining supply using totalSupply + yieldFeeBalance and deposit() should validate the limit as well.

    function _twabSupplyLimit(uint256 _totalSupply) internal pure returns (uint256) {
        unchecked {
            return type(uint96).max - _totalSupply - yieldFeeBalance;
        }
    }

Assessed type

Invalid Validation