Deep Seaweed Haddock - The created vault is revokeProxy, not includes non revokable.

[sherlock-admin3]

Deep Seaweed Haddock

Low/Info

The created vault is revokeProxy, not includes non revokable.

Summary

The function createVault in CuratedVaultFactory directly creates a vault by RevokableBeaconProxy, which makes the new vault revocable regardless of whether revokeProxy is true or false.

Root Cause

In CuratedVaultFactory.sol:50, there is a missing check whether or not revokeProxy is true. https://github.com/sherlock-audit/2024-06-new-scope/blob/main/zerolend-one/contracts/core/vaults/CuratedVaultFactory.sol#L48

Internal pre-conditions

No response

External pre-conditions

No response

Attack Path

No response

Impact

1. The created vault is revokeProxy when creating a new vault by setting the revokeProxy as false.

PoC

Under CuratedVaultFactoryTest.sol add POC test for judge revokeProxy

function testPOCCreateVault(
    address initialOwner,
    uint256 initialTimelock,
    string memory name,
    string memory symbol,
    bytes32 salt,
    bool revokeProxy 
  ) public {
    vm.assume(address(initialOwner) != address(0));
    vm.assume(initialTimelock >= 1 days);

    initialTimelock = bound(initialTimelock, 2 days, 2 weeks);

    defaultVaultParams.proxyAdmin = initialOwner;
    defaultVaultParams.admins[0] = initialOwner;
    defaultVaultParams.timelock = initialTimelock;
    defaultVaultParams.name = name;
    defaultVaultParams.symbol = symbol;
    defaultVaultParams.salt = salt;
   // if revokeProxy is true, the owner as vaultFactory like pookFactory
    if (revokeProxy) {
      initialOwner = address(vaultFactory);
      defaultVaultParams.admins[0] = address(vaultFactory);
    }
    defaultVaultParams.revokeProxy = revokeProxy;
    bytes32 initCodeHash = hashInitCode(type(RevokableBeaconProxy).creationCode, abi.encode(address(vaultFactory), initialOwner));
    address expectedAddress = computeCreate2Address(salt, initCodeHash, address(vaultFactory));

    // vm.expectEmit(address(vaultFactory));
    // emit CuratedEventsLib.VaultCreated(ICuratedVault(expectedAddress), uint256(2), defaultVaultParams, address(this));

    ICuratedVault vault = vaultFactory.createVault(defaultVaultParams);

    assertEq(expectedAddress, address(vault), 'computeCreate2Address');
    assertTrue(vaultFactory.isVault(address(vault)), 'isvault');
    if (revokeProxy) {
      assertTrue(vault.isOwner(address(vaultFactory)), 'owner');
    } else {
      // if the revokeProxy is false when createVault. the owner is the initialOwner
      assertTrue(vault.isOwner(address(initialOwner)), 'owner');
    }

    assertEq(vault.timelock(), initialTimelock, 'timelock');
    assertEq(vault.asset(), address(loanToken), 'asset');
    assertEq(vault.name(), name, 'name');
    assertEq(vault.symbol(), symbol, 'symbol');
  }

Mitigation

Can reference PoolFactory.sol:70, create Vault should depend on the revokeProxy

function createVault(InitVaultParams memory p) external returns (ICuratedVault vault) {
    // create the vault
    if (p.revokeProxy) {
      vault = ICuratedVault(address(new RevokableBeaconProxy{salt: p.salt}(address(this), address(this))));
      IRevokableBeaconProxy(address(vault)).revokeBeacon();
    } else {
      if (p.proxyAdmin == address(0)) p.proxyAdmin = msg.sender;
      vault = ICuratedVault(address(new RevokableBeaconProxy{salt: p.salt}(address(this), p.proxyAdmin)));
    }

    emit CuratedEventsLib.VaultCreated(vault, vaults.length, p, msg.sender);

    vault.initialize(p.admins, p.curators, p.guardians, p.allocators, p.timelock, p.asset, p.name, p.symbol);

    // track the vault
    vaults.push(vault);
    isVault[address(vault)] = true;
  }