GasVault Static Analysis Findings

GVT-01S: Deprecated Native Asset Transfers

Type	Severity	Location
Language Specific		GasVault.sol:L87, L96

Description:

The linked statements perform low-level native asset transfers 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 were susceptible to this issue and would cause native transfers to fail if sent to a new address.

Example:

Copy
87payable(to).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 (200f275c40cbd4798f4a416c044ea726755d4741):

Both low-level transfer statements were replaced by sendValue invocations ensuring that transfers will never cease functioning due to updates in the underlying EVM.

GVT-02S: Inexistent Sanitization of Input Addresses

Type	Severity	Location
Input Sanitization		GasVault.sol:L38-L50

Description:

The linked function(s) accept address arguments yet do not properly sanitize them.

Impact:

The presence of zero-value addresses, especially in constructor implementations, can cause the contract to be permanently inoperable. These checks are advised as zero-value inputs are a common side-effect of off-chain software related bugs.

Example:

Copy
38function initialize(
39    address _orchestrator,
40    address _stratRegistry,
41    address _vaultRegistry,
42    address _governance
43) public initializer {
44    __Ownable_init();
45    __UUPSUpgradeable_init();
46    orchestrator = IOrchestrator(_orchestrator);
47    strategyRegistry = IStrategyRegistry(_stratRegistry);
48    vaultRegistry = IVaultRegistry(_vaultRegistry);
49    governance = _governance;
50}

Recommendation:

We advise some basic sanitization to be put in place by ensuring that each address specified is non-zero.

Alleviation (200f275c40cbd4798f4a416c044ea726755d4741):

All four arguments of the initialize function are now properly sanitized against the zero-address.