code423n4 commented 1 year ago

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. When _liquidationPair calls liquidate, the call can get frontrun by the contract owner calling _setYieldFeePercentage, which changes the percentage of _amountOut that is passed to _increaseYieldFeeBalance and in doing directly changes the value of _yieldFeeTotalSupply.

Proof of Concept

The call to _increaseYieldFeeBalance, which increases the value of _yieldFeeTotalSupply, takes place in liquidate

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

But setYieldFeePercentage can be called at any time by the owner, such as by frontrunning, allowing the amount of value accumulated for the _yieldFeeRecipient to be fully controlled by the owner.

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

Tools Used

Manual review

Recommended Mitigation Steps

Create a new state variable _lastYieldUpdate that stores the current block.timestamp inside of setYieldFeePercentage. A check can then be introduced in liquidate to revert or take other actions if block.timestamp == _lastYieldUpdate, which will prevent a user from becoming a victim of frontrunning from the contract owner.