code423n4 commented 1 year ago

Lines of code

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

Vulnerability details

Impact

_yieldFeeRecipient does not receive the yield fee

Yield fee is not deducted from user's received shares

Proof of Concept

Function liquidate of Vault is described as User provides prize tokens and receives in exchange Vault shares.. Basically, user will give prize token, then they receive back vault's shares after paying a fee configured by the vault. Below is the implementation function liquidate:

function liquidate(
address _account,
address _tokenIn,
uint256 _amountIn,
address _tokenOut,
uint256 _amountOut
) public virtual override returns (bool) {
_requireVaultCollateralized();
if (msg.sender != address(_liquidationPair))
  revert LiquidationCallerNotLP(msg.sender, address(_liquidationPair));
if (_tokenIn != address(_prizePool.prizeToken()))
  revert LiquidationTokenInNotPrizeToken(_tokenIn, address(_prizePool.prizeToken()));
if (_tokenOut != address(this))
  revert LiquidationTokenOutNotVaultShare(_tokenOut, address(this));
if (_amountOut == 0) revert LiquidationAmountOutZero();

uint256 _liquidableYield = _liquidatableBalanceOf(_tokenOut);
if (_amountOut > _liquidableYield)
  revert LiquidationAmountOutGTYield(_amountOut, _liquidableYield);

_prizePool.contributePrizeTokens(address(this), _amountIn);

if (_yieldFeePercentage != 0) {
  _increaseYieldFeeBalance(
    (_amountOut * FEE_PRECISION) / (FEE_PRECISION - _yieldFeePercentage) - _amountOut
  );
}

uint256 _vaultAssets = IERC20(asset()).balanceOf(address(this));

if (_vaultAssets != 0 && _amountOut >= _vaultAssets) {
  _yieldVault.deposit(_vaultAssets, address(this));
}

_mint(_account, _amountOut);

return true;
}

As you can see, the code does not deduct the fee and user receive full _amountOut shares, also the fee is not transferred to _yieldFeeRecipient. In fact, variable _yieldFeeRecipient is not used in contract's at all other than the functions to set it.

Below is a POC for the above example, for ease of testing, let's place this test case in file vault/test/unit/Vault/Liquidate.t.sol under contract VaultLiquidateTest, then test it using command: forge test --match-path test/unit/Vault/Liquidate.t.sol --match-test testYieldFeeIsNotTransferred -vvvv

function testYieldFeeIsNotTransferred() external {
    _setLiquidationPair();

    // set yield fee percentage and recipient
    vault.setYieldFeePercentage(200000000);
    address newYieldFeeRecipient = address(0x9999);
    vault.setYieldFeeRecipient(newYieldFeeRecipient);

    underlyingAsset.mint(address(this), 1000e18);
    _sponsor(underlyingAsset, vault, 1000e18, address(this));

    // Mint some yield
    underlyingAsset.mint(address(yieldVault), 10e18);

    // Mint some prize tokens for alice
    prizeToken.mint(alice, 1000e18);

    // Alice liquidate (deposit prize tokens in exchange for vault shares)
    uint256 aliceShares = 1e18; // this is also the desired share alice want to receive
    vm.prank(address(liquidationPair));
    vault.liquidate(
      alice,
      address(prizeToken),
      1e18, // this is tokens in
      address(vault),
      aliceShares // amountOut
    );

    // Alice receive full amountOut
    assertEq(vault.balanceOf(alice), aliceShares);

    // _yieldFeeRecipient receives no share

    assertEq(newYieldFeeRecipient, vault.yieldFeeRecipient());
    assertEq(vault.balanceOf(newYieldFeeRecipient), 0);

  }