Incorrect yield fee calculation will charge more on transferring tokens out than expected

[c4-bot-7]

Lines of code

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

Vulnerability details

Impact

The protocol has a fee mechanism which accumulates fees when tokens are transferred out of the protocol. However the yield calculation is wrong and charges more fee than anticipated.

Proof of Concept

At FEE_PERCISION = 1e9 Assume yieldFeePercentage_ is 1e8 or in other words 10% fee Assume _amountOut is 1e18

The fee should be equal to 1e18 / 10 or 1e17.

However calculated by the current formula the result is 1.1111(1)e17: _yieldFee = (_amountOut * FEE_PRECISION) / (FEE_PRECISION - _yieldFeePercentage) - _amountOut;

(1e18 * 1e9) / (1e9 - 1e8) - 1e18 1e27 / 10e8 - 1e18 1.1111111e+18 - 1e18 1.1111111e17

Tools Used

Manual Review

Recommended Mitigation Steps

In PrizeVault::transferTokensOut do the following change

...
        uint256 _availableYield = availableYieldBalance();
        uint32 _yieldFeePercentage = yieldFeePercentage;

        // Determine the proportional yield fee based on the amount being liquidated:
        uint256 _yieldFee;
        if (_yieldFeePercentage != 0) {
            // The yield fee is calculated as a portion of the total yield being consumed, such that
            // `total = amountOut + yieldFee` and `yieldFee / total = yieldFeePercentage`.
-            _yieldFee = (_amountOut * FEE_PRECISION) / (FEE_PRECISION - _yieldFeePercentage) - _amountOut;
+            _yieldFee = (_amountOut * _yieldFeePercentage) / FEE_PRECISION;
        }

        // Ensure total liquidation amount does not exceed the available yield balance:
        if (_amountOut + _yieldFee > _availableYield) {
            revert LiquidationExceedsAvailable(_amountOut + _yieldFee, _availableYield);
        }
...

Assessed type

Math

[c4-bot-10]

Withdrawn by yotov721