code423n4 commented 1 year ago

Lines of code

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

Vulnerability details

Impact

In the Vault.constructor there is no check to verify that the user passed-in _asset for the standard vault construction and the underlying asset of the _yieldVault are the same. If standard vault is constructed with a different _asset to the one used in the yieldVault, the deposit, mint, withdraw and redeem functionalities will revert. Hence a redeployment of the Vault contract will be needed with the correct underlying asset.

If any _asset is sent to the contract directly, those assets will be permanently locked inside the Vault contract since there is no mechanism to recover the locked assets.

Proof of Concept

  constructor(
    IERC20 asset_,
    string memory name_,
    string memory symbol_,
    TwabController twabController_,
    IERC4626 yieldVault_,
    PrizePool prizePool_,
    address claimer_,
    address yieldFeeRecipient_,
    uint256 yieldFeePercentage_,
    address owner_
  ) ERC4626(asset_) ERC20(name_, symbol_) ERC20Permit(name_) Ownable(owner_) {
    if (address(twabController_) == address(0)) revert TwabControllerZeroAddress();
    if (address(yieldVault_) == address(0)) revert YieldVaultZeroAddress();
    if (address(prizePool_) == address(0)) revert PrizePoolZeroAddress();
    if (address(owner_) == address(0)) revert OwnerZeroAddress();

    _twabController = twabController_;
    _yieldVault = yieldVault_;
    _prizePool = prizePool_;

    _setClaimer(claimer_);
    _setYieldFeeRecipient(yieldFeeRecipient_);
    _setYieldFeePercentage(yieldFeePercentage_);

    _assetUnit = 10 ** super.decimals();

    // Approve once for max amount
    asset_.safeApprove(address(yieldVault_), type(uint256).max);

    emit NewVault(
      asset_,
      name_,
      symbol_,
      twabController_,
      yieldVault_,
      prizePool_,
      claimer_,
      yieldFeeRecipient_,
      yieldFeePercentage_,
      owner_
    );
  }

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

Tools Used

Manual Review and VSCode

Recommended Mitigation Steps

Hence it is recommended to check the _asset underlying token is the same as the underlying token of the _yieldVault inside the Vault.constructor function.

Furthermore it is recommended to include a function to recover the directly sent assets to the vault (which are locked) to an admin controlled reserve. This function should be only callable by the onlyOwner.