VaultFactory Static Analysis Findings

VFY-01S: Deprecated Native Asset Transfer

Type	Severity	Location
Language Specific		VaultFactory.sol:L171

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
171if (msg.value > mintFee) payable(msg.sender).transfer(msg.value - mintFee);

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 (7a4e1dc948e94ded7385dbb74818bcf93ecc207c):

The native fund transfer performed in the referenced statement has been properly replaced by OpenZeppelin's Address::sendValue function, ensuring that the transfer will fail irrespective of what blockchain the contract is deployed in.