yieldFeeBalance wouldn't be claimed after calling transferTokensOut().

[c4-bot-9]

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

[c4-pre-sort]

raymondfam marked the issue as insufficient quality report

[c4-pre-sort]

raymondfam marked the issue as duplicate of #319

[raymondfam]

On top of the comments on #319, _tokenOut is just a specifier whether to mint shares or to withdraw assets to the receiver. And the yieldFee added to yieldFeeBalance will be incorporated into totalDebt. The yield fee recipient will be able to claim it anytime unrestricted via the pull method.

[c4-judge]

hansfriese marked the issue as not a duplicate

[hansfriese]

The impact is the same as #91 but the flaw still exists after mitigating #91 because liquidatableBalanceOf() doesn't use the newly accumulated yield fees while checking _twabSupplyLimit.

[c4-judge]

hansfriese marked the issue as satisfactory

[trmid]

Similar to #91, this issue outlines the need for the TWAB supply limit checks to account for the yield fee balance so that the entire yield fee balance is always available to be realized as shares.

[c4-sponsor]

trmid (sponsor) confirmed

[c4-judge]

hansfriese marked the issue as selected for report

[trmid]

mitigation: GenerationSoftware/pt-v5-vault#93