MetavisorRegistry Manual Review Findings

MRY-01M: Inexistent Sanitization of Swap Percentage

Type	Severity	Location
Input Sanitization		MetavisorRegistry.sol:L48

Description:

While the _swapPercentage is adequately sanitized during its setSwapPercentage setter function, it is not validated in the constructor of the contract.

Impact:

An incorrect swapPercentage can cause a casting overflow to occur in the MetavisorManagedVault::rescale implementation as evidenced in a separate exhibit.

Example:

Copy
45constructor(address _uniswapFactory, address _weth, uint256 _swapPercentage) {
46    uniswapFactory = IUniswapV3Factory(_uniswapFactory);
47    weth = IWETH9(_weth);
48    swapPercentage = _swapPercentage;
49
50    vaultMaster = address(new MetavisorManagedVault());
51
52    isOperator[msg.sender] = true;
53}

Recommendation:

We advise the same DENOMINATOR-based sanitization applied in setSwapPercentage to be applied in the constructor of the contract, ensuring that the swap percentage has been properly defined.

Alleviation:

The initial _swapPercentage specified in the contract's constructor is now properly validated as advised.

MRY-02M: Inexistent Sanitization of Pool Validity

Type	Severity	Location
Input Sanitization		MetavisorRegistry.sol:L90

Description:

The contract contains an IUniswapV3Factory member yet does not sanitize the input _pool variables that are utilized during the construction of a vault.

Impact:

As deployments of vaults are expected to be "open" at a later point in the project's lifetime, there is no security measure in place to prohibit a malicious _pool implementation and thus a compromise of the MetavisorManagedVault implementation's proper operation.

Example:

Copy
89function deployVault(
90    address _pool,
91    VaultType _vaultType,
92    VaultSpec calldata _vaultSpec
93) external returns (address) {
94    if (!openDeploy) {
95        _checkOwner();
96    }
97
98    if (deployedVaults[_pool][_vaultType] != address(0)) {
99        revert Errors.AlreadyDeployed();
100    }
101
102    address _newVault = Clones.cloneDeterministic(
103        vaultMaster,
104        keccak256(abi.encodePacked(_pool, _vaultType))
105    );
106    setVaultSpec(_newVault, _vaultSpec);
107
108    MetavisorManagedVault(payable(_newVault)).__MetavisorManagedVault__init(
109        address(this),
110        _pool,
111        _vaultType
112    );
113    deployedVaults[_pool][_vaultType] = _newVault;
114    allVaults.push(_newVault);
115
116    emit VaultCreated(_pool, _vaultType, _newVault);
117
118    return _newVault;
119}

Recommendation:

We advise the code to properly validate that the _pool is an active UniswapV3 pool by querying the IUniswapV3Factory::getPool function based on data extracted from the _pool itself cast to an IUniswapV3Pool interface.

Alleviation:

The validation of a _pool has been delegated to the constructor of the MetavisorBaseVault that the MetavisorManagedVault inherits from, adequately validating that the _pool supplied in deployVault represents the correct pool address for the configuration provided.