`liquidate` calls `_increaseYieldFeeBalance` with incorrect value

[code423n4]

Lines of code

https://github.com/GenerationSoftware/pt-v5-vault/blob/b1deb5d494c25f885c34c83f014c8a855c5e2749/src/Vault.sol#L573-L575

Vulnerability details

Impact

liquidate is the only function where _increaseYieldFeeBalance is called. _increaseYieldFeeBalance increases the value of _yieldFeeTotalSupply by some number of shares. The argument passed to _increaseYieldFeeBalance should be a number of shares. The value that is passed to _increaseYieldFeeBalance has units of shares, because _amountOut has units of shares, but the calculation is incorrect and can be manipulated.

Proof of Concept

Take the value that is passed to _increaseYieldFeeBalance

https://github.com/GenerationSoftware/pt-v5-vault/blob/b1deb5d494c25f885c34c83f014c8a855c5e2749/src/Vault.sol#L573-L575

The exact value is (_amountOut * FEE_PRECISION) / (FEE_PRECISION - _yieldFeePercentage) - _amountOut, but a simplified analogy will be easier to understand first. Take the formula ( _amountOut * (100 / (100 - x)) ) - _amountOut. The fraction 100 / (100 - x) has the smallest value of 1 when x = 0 and the greatest value of 100 when x = 99.

Returning to the original value of (_amountOut * FEE_PRECISION) / (FEE_PRECISION - _yieldFeePercentage) - _amountOut, the maximum value is when _yieldFeePercentage = FEE_PRECISION - 1 so that the resulting value is (_amountOut * FEE_PRECISION) - _amountOut which can be simplified to _amountOut * (FEE_PRECISION - 1). Because FEE_PRECISION is a constant with value 1e9, this means _yieldFeeTotalSupply can increase by 9 orders of magnitude greater than the _amountOut number of shares passed to liquidate.

Tools Used

Manual review

Recommended Mitigation Steps

Consider passing the value _amountOut * (FEE_PRECISION - _yieldFeePercentage) / FEE_PRECISION to _increaseYieldFeeBalance.

Assessed type

Math

[c4-judge]

Picodes marked the issue as primary issue

[c4-sponsor]

asselstine marked the issue as sponsor confirmed

[c4-judge]

Picodes marked the issue as duplicate of #124

[c4-judge]

Picodes marked the issue as satisfactory

[c4-judge]

Picodes marked the issue as unsatisfactory: Invalid