PrizeVault:transferTokensOut miscalculates the yield fee

Summary

The transferTokensOut at the PrizeVault contract currently calculates yield fee percentages wrong.

Vulnerability Detail

The transferTokensOut function currently does the following calculation to get yield fee:

function transferTokensOut(
        address /* sender */,
        address _receiver,
        address _tokenOut,
        uint256 _amountOut
    ) {
    ...
    // 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;
        }
    ...

    }

The logic is incorrect. If we break down both mathematical expressions at the developer comment, we can see the written expression is impossible:

// `total = amountOut + yieldFee` and `yieldFee / total = yieldFeePercentage`.

Given the relationships:

total = amountOut + yieldFee yieldFee / total = yieldFeePercentage

From these relationships, we can deduce the correct formula.

Derivation:

From the second relationship, we get: yieldFee = yieldFeePercentage * total

Substitute the expression for total: yieldFee = yieldFeePercentage * (amountOut + yieldFee)

Rearrange to solve for yieldFee: yieldFee = yieldFeePercentage amountOut + yieldFeePercentage yieldFee

Isolate yieldFee: yieldFee - yieldFeePercentage yieldFee = yieldFeePercentage amountOut

Factor out yieldFee: yieldFee (1 - yieldFeePercentage) = yieldFeePercentage amountOut

Finally, solve for yieldFee: yieldFee = (yieldFeePercentage * amountOut) / (1 - yieldFeePercentage)

Impact

As we can see, the current expression doesn't multiply the numerator by yieldFeePercentage. All YieldFee calculations at the transferTokensOut function yield wrong results

Code Snippet

https://github.com/sherlock-audit/2024-05-pooltogether/blob/1aa1b8c028b659585e4c7a6b9b652fb075f86db3/pt-v5-vault/src/PrizeVault.sol#L731

Tool used

Manual Review

Recommendation

Make sure to factor yieldFeePercentage in when calculating the yield fees. The following snippet contains a possible correction for the issue:

function transferTokensOut(
        address /* sender */,
        address _receiver,
        address _tokenOut,
        uint256 _amountOut
    ) {
    ...
    uint256 _yieldFee;
    if (_yieldFeePercentage != 0) {
        // Calculate yield fee using the derived formula:
        // yieldFee = (yieldFeePercentage * amountOut) / (1 - yieldFeePercentage)

        // Applying FEE_PRECISION to handle the percentage calculation accurately:
        _yieldFee = (_yieldFeePercentage * _amountOut ) / (FEE_PRECISION - _yieldFeePercentage);
    }
    ...
}

sherlock-audit / 2024-05-pooltogether-judging

jovi - PrizeVault:transferTokensOut miscalculates the yield fee #85