Loss of fees due to multiple issues with `Vault.mintYieldFee()`

[code423n4]

Lines of code

https://github.com/GenerationSoftware/pt-v5-vault/blob/b1deb5d494c25f885c34c83f014c8a855c5e2749/src/Vault.sol#L573 https://github.com/GenerationSoftware/pt-v5-vault/blob/b1deb5d494c25f885c34c83f014c8a855c5e2749/src/Vault.sol#L394-L399 https://github.com/GenerationSoftware/pt-v5-vault/blob/b1deb5d494c25f885c34c83f014c8a855c5e2749/src/Vault.sol#L704-L706 https://github.com/GenerationSoftware/pt-v5-vault/blob/b1deb5d494c25f885c34c83f014c8a855c5e2749/src/Vault.sol#L1229-L1231

Vulnerability details

Impact

The function Vault.mintYieldFee() is missing access control and can be called by anybody. It also allows to specify any _recipient to receive the shares for the yield fee. It ignores the state variable Vault._yieldFeeRecipient, which is the address of the yield fee recipient, that can be set via the constructor of the Vault.sol contract or via Vault.setYieldFeeRecipient() by the owner of the vault.

All these issues potentially lead to undesired behavior:

1. When Vault.liquidate() is called, the function Vault._increaseYieldFeeBalance gets called which increases the _yieldFeeTotalSupply. After Vault.liquidate() was called, a malicious actor can call Vault.mintYieldFee() before the liquidator is calling the function, in order to frontrun and steal the shares of the yield fee (_yieldFeeTotalSupply), by specifying the malicious actor's address for the _recipient function parameter and minting the shares for themselves. Vault._yieldFeeRecipient gets ignored.

2. Also the liquidator could send the shares for the yield fee anywhere when calling Vault.mintYieldFee(), by specifying any address for the function parameter _recipient. Vault._yieldFeeRecipient gets ignored.

3. The owner of the Vault.sol contract calls Vault.setYieldFeeRecipient() in order to specify who should receive the shares for the yield fee. But when Vault.mintYieldFee() is called afterwards, the value Vault._yieldFeeRecipient that the owner set is ignored, and the shares for the yield fee are being sent to some other recipient potentially.

Because of these issues, yield fee shares could be lost: either because they could be stolen by a malicious actor or they could be send to an inadvertent recipient by a liquidator. The Vault owner can't influence who will receive the shares for the yield fees.

Proof of Concept

Here is a POC:

https://gist.github.com/zzzitron/7ec08fc8428ebe86e193dc35717a997f

Tools Used

Manual review

Recommended Mitigation Steps

Consider removing the function parameter _recipient from Vault.mintYieldFee() and send the shares to the specified Vault._yieldFeeRecipient of the Vault.sol contract.

Or consider introducing access control to Vault.mintYieldFee():

// Vault.sol
394  function mintYieldFee(uint256 _shares, address _recipient) external {
395    if (msg.sender != address(_liquidationPair)) {
396        // revert

Assessed type

Access Control

[c4-judge]

Picodes marked the issue as duplicate of #396

[c4-judge]

Picodes changed the severity to 3 (High Risk)

[c4-judge]

Picodes marked the issue as satisfactory