`mintYieldFee` doesn't mint yield fee to the `_yieldFeeRecipient` in `Vault` contract

[code423n4]

Lines of code

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

Vulnerability details

Impact

The mintYieldFee function inside the Vault contract can be called by anyone to transfer the accrued yield fees shares to a given _recipient address (set by the user), but the function is normally supposed to transfer those fees to the _yieldFeeRecipient set by the contract owner.

Proof of Concept

The issue occurs in the mintYieldFee function below :

File: Vault.sol Line 394-402

function mintYieldFee(uint256 _shares, address _recipient) external {
    _requireVaultCollateralized();
    if (_shares > _yieldFeeTotalSupply) revert YieldFeeGTAvailable(_shares, _yieldFeeTotalSupply);

    _yieldFeeTotalSupply -= _shares; 
    // @audit The fees shares should be minted to `_yieldFeeRecipient`
    _mint(_recipient, _shares);

    emit MintYieldFee(msg.sender, _recipient, _shares);
}

As you can see from the code above, the function has no access control (modifier or check on the caller), it allows the caller to give any _recipient address which the fee shares get minted to (when the vault is collateralized, of course).

But from the comments we can find that all the fees captured should go to the _yieldFeeRecipient and don't to a random user :

/// @notice Address of the yield fee recipient that receives the fee amount when yield is captured.
address private _yieldFeeRecipient;

The impact of this issue is that all the accrued fee shares are distributed to random users who make a call to the mintYieldFee function instead of the _yieldFeeRecipient. And in order to make sure that the fees are minted to the _yieldFeeRecipient, the protocol devs would have to monitor at each moment when the vault is collateralized and try to be the first to call the mintYieldFee function which not suitable and maybe impossible.

Tools Used

Manual review

Recommended Mitigation Steps

To solve this issue i recommend to remove the _recipient address argument from the mintYieldFee function and mint directly to the _yieldFeeRecipient address.

The code should become :

function mintYieldFee(uint256 _shares) external {
    _requireVaultCollateralized();
    if (_shares > _yieldFeeTotalSupply) revert YieldFeeGTAvailable(_shares, _yieldFeeTotalSupply);

    _yieldFeeTotalSupply -= _shares; 
    // @audit any user that call the function will mint to `_yieldFeeRecipient`
    _mint(_yieldFeeRecipient, _shares);

    emit MintYieldFee(msg.sender, _yieldFeeRecipient, _shares);
}

Assessed type

Token-Transfer

[c4-judge]

Picodes marked the issue as duplicate of #406

[c4-judge]

Picodes marked the issue as not a duplicate

[c4-judge]

Picodes marked the issue as duplicate of #396

[c4-judge]

Picodes marked the issue as satisfactory