MetaProxyDeployer Manual Review Findings

MPD-01M: Inexistent Allocation of Memory

Type	Severity	Location
Language Specific		MetaProxyDeployer.sol:L16

Description:

The MetaProxyDeployer contract will load the free memory pointer to start constructing its deployment payload, however, the code will never offset the free memory pointer to properly indicate the memory space has been allocated.

Impact:

The memory space utilized by the MetaProxyDeployer contract for the proxy's bytecode is never indicated as reserved in the free memory pointer, indicating non-standard usage of memory.

Example:

Copy
8/// @dev Creates a proxy for `targetContract` with metadata from `metadata`. Code modified from EIP-3448 reference implementation: https://eips.ethereum.org/EIPS/eip-3448
9/// @return addr A non-zero address if successful.
10function deployMetaProxy(address targetContract, bytes memory metadata) internal returns (address addr) {
11    // the following assembly code (init code + contract code) constructs a metaproxy.
12    assembly {
13        let offset := add(metadata, 32)
14        let length := mload(metadata)
15        // load free memory pointer as per solidity convention
16        let start := mload(64)
17        // keep a copy
18        let ptr := start
19        // deploy code (11 bytes) + first part of the proxy (21 bytes)
20        mstore(ptr, 0x600b380380600b3d393df3363d3d373d3d3d3d60368038038091363936013d73)
21        ptr := add(ptr, 32)
22
23        // store the address of the contract to be called
24        mstore(ptr, shl(96, targetContract))
25        // 20 bytes
26        ptr := add(ptr, 20)
27
28        // the remaining proxy code...
29        mstore(ptr, 0x5af43d3d93803e603457fd5bf300000000000000000000000000000000000000)
30        // ...13 bytes
31        ptr := add(ptr, 13)
32
33        // copy the metadata
34        {
35            for { let i := 0 } lt(i, length) { i := add(i, 32) } { mstore(add(ptr, i), mload(add(offset, i))) }
36        }
37        ptr := add(ptr, length)
38
39        // The size is deploy code + contract code + calldatasize - 4.
40        addr := create(0, start, sub(ptr, start))
41    }
42}

Recommendation:

We advise the free memory pointer to be updated properly in adherence to Solidity conventions.

To note, a vulnerability does not manifest in the present code as the ptr variable that would be corrupted is atomically utilized within the MetaProxyDeployer::deployMetaProxy. Despite of this, we still advise the memory pointer to be updated accordingly to prevent any issues arising in future updates of the code.

Alleviation (fb2dd77a6f):

The Euler Finance team opted to acknowledge this exhibit based on the fact that they have copied their code from the reference code of the EIP-3448 page.

While we understand the exhibit's acknowledgement, we would like to clarify that an EIP's example implementation is not meant to be secure but rather illustrate how the code should function.

Alleviation (0f2192ac81):

The Euler Finance team opted to create their own MetaProxyDeployer implementation based on the definition of the EIP-3448 standard, significantly increasing the legibility of the implementation.

As the code no longer reserves memory from the free memory pointer, we consider this exhibit no longer applicable.

MPD-02M: Inexistent Appendment of Metadata Length (EIP-3448 Discrepancy)

Type	Severity	Location
Standard Conformity		MetaProxyDeployer.sol:L37

Description:

Per the EIP-3448 standard definition as well as reference implementation, the trailing 32 bytes of the contract's bytecode MUST (RFC-2119) indicate the length of the metadata.

In the bytecode constructed by the MetaProxyDeployer, no size is appended at the end of the bytecode thereby generating code that is non-compliant with the EIP-3448 standard.

Impact:

The MetaProxyDeployer contract generates bytecode that is not compliant with the EIP-3448 standard.

As this discrepancy is acknowledged in the contract's documentation, we consider the exhibit to be informational. To note, we still advise the size of the data to be appended as the EIP-3448 terminology in use is presently misleading given that the EIP-3448 standard is closely correlated to the appendment of the metadata size per its MUST RFC-2119 terminology.

Example:

Copy
33// copy the metadata
34{
35    for { let i := 0 } lt(i, length) { i := add(i, 32) } { mstore(add(ptr, i), mload(add(offset, i))) }
36}
37ptr := add(ptr, length)
38
39// The size is deploy code + contract code + calldatasize - 4.
40addr := create(0, start, sub(ptr, start))

Recommendation:

We advise the ptr to be written to after being offset by the metadata length to store the length value itself, ensuring that the generated bytecode is compliant with the EIP-3448 standard.

Alleviation (fb2dd77a6ff9b7f710edb48e7eb5437e0db4fc1a):

The Euler Finance team clarified that the contract is meant to be inspired by the EIP-3448 standard rather than adhering to it, and performed documentational adjustments to imply as such.

Based on these changes, we consider this exhibit to be properly addressed as the contract's purpose is no longer up to interpretation.

MPD-03M: Inexistent Handling of Creation Failure

Type	Severity	Location
Language Specific		MetaProxyDeployer.sol:L40

Description:

The create EVM operation code may yield the zero-address in case of failure, however, this is not explicitly handled by the MetaProxyDeployer::deployMetaProxy function nor the GenericFactory::createProxy function it is invoked in.

Impact:

Although unlikely, any failure of the MetaProxyDeployer::deployMetaProxy function will not be explicitly handled and will instead result in a failure implicitly in the GenericFactory::createProxy function and specifically the IComponent::initialize call it performs.

As the code is secure as-is, a severity of informational was deemed appropriate.

Example:

Copy
8/// @dev Creates a proxy for `targetContract` with metadata from `metadata`. Code modified from EIP-3448 reference implementation: https://eips.ethereum.org/EIPS/eip-3448
9/// @return addr A non-zero address if successful.
10function deployMetaProxy(address targetContract, bytes memory metadata) internal returns (address addr) {
11    // the following assembly code (init code + contract code) constructs a metaproxy.
12    assembly {
13        let offset := add(metadata, 32)
14        let length := mload(metadata)
15        // load free memory pointer as per solidity convention
16        let start := mload(64)
17        // keep a copy
18        let ptr := start
19        // deploy code (11 bytes) + first part of the proxy (21 bytes)
20        mstore(ptr, 0x600b380380600b3d393df3363d3d373d3d3d3d60368038038091363936013d73)
21        ptr := add(ptr, 32)
22
23        // store the address of the contract to be called
24        mstore(ptr, shl(96, targetContract))
25        // 20 bytes
26        ptr := add(ptr, 20)
27
28        // the remaining proxy code...
29        mstore(ptr, 0x5af43d3d93803e603457fd5bf300000000000000000000000000000000000000)
30        // ...13 bytes
31        ptr := add(ptr, 13)
32
33        // copy the metadata
34        {
35            for { let i := 0 } lt(i, length) { i := add(i, 32) } { mstore(add(ptr, i), mload(add(offset, i))) }
36        }
37        ptr := add(ptr, length)
38
39        // The size is deploy code + contract code + calldatasize - 4.
40        addr := create(0, start, sub(ptr, start))
41    }
42}

Recommendation:

Given that the GenericFactory::createProxy function expects the BeaconProxy failure to lead to a revert, we advise the MetaProxyDeployer::deployMetaProxy function to revert as well if the yielded address iszero.

Alleviation (fb2dd77a6f):

The Euler Finance team opted to acknowledge this exhibit based on the fact that they have copied their code from the reference code of the EIP-3448 page.

While we understand the exhibit's acknowledgement, we would like to clarify that an EIP's example implementation is not meant to be secure but rather illustrate how the code should function.

Alleviation (0f2192ac81):

The Euler Finance team opted to create their own MetaProxyDeployer implementation based on the definition of the EIP-3448 standard, significantly increasing the legibility of the implementation.

The latest MetaProxyDeployer::deployMetaProxy function implementation will properly validate that the contract was deployed by ensuring the addr is non-zero, alleviating this exhibit.