MetavisorManagedVault Static Analysis Findings

MMV-01S: Deprecated Native Asset Transfer

Type	Severity	Location
Language Specific		MetavisorManagedVault.sol:L315

Description:

The linked statement performs a low-level native asset transfer via the transfer function exposed by the address payable data type.

Impact:

As new EIPs such as EIP-2930 are introduced to the blockchain, gas costs can change and the transfer instruction of Solidity specifies a fixed gas stipend that is prone to failure should such changes be integrated to the blockchain the contract is deployed in. A prime example of this behaviour are legacy versions of Gnosis which were susceptible to this issue and would cause native transfers to fail if sent to a new address.

Example:

Copy
315payable(recipient).transfer(amount);

Recommendation:

We advise alternative ways of transferring assets to be utilized instead, such as OpenZeppelin's Address.sol library and in particular the sendValue method exposed by it. If re-entrancies are desired to be prevented based on gas costs, we instead advise a mechanism to be put in place that either credits an account with a native balance they can withdraw at a secondary transaction or that performs the native asset transfers at the end of the top-level transaction's execution.

Alleviation:

A low-level call instruction is now employed instead, ensuring the funds are successfully transmitted by allocating additional gas to their transfer.

MMV-02S: Improper Invocations of EIP-20 transfer / transferFrom

Type	Severity	Location
Standard Conformity		MetavisorManagedVault.sol:L60, L63, L73, L76, L317, L335, L338

Description:

The linked statements do not properly validate the returned bool values of the EIP-20 standard transfer & transferFrom functions. As the standard dictates, callers must not assume that false is never returned.

Impact:

If the code mandates that the returned bool is true, this will cause incompatibility with tokens such as USDT / Tether as no such bool is returned to be evaluated causing the check to fail at all times. On the other hand, if the token utilized can return a false value under certain conditions but the code does not validate it, the contract itself can be compromised as having received / sent funds that it never did.

Example:

Copy
60token0.transfer(msg.sender, amount0Owed);

Recommendation:

Since not all standardized tokens are EIP-20 compliant (such as Tether / USDT), we advise a safe wrapper library to be utilized instead such as SafeERC20 by OpenZeppelin to opportunistically validate the returned bool only if it exists in each instance.

Alleviation:

All referenced invocations of transfer and transferFrom instructions have been adequately replaced by their safe-prefixed counterparts, alleviating this exhibit.