GenericFactory Manual Review Findings

GFY-01M: Potentially Dangerous Push Ownership Pattern

Type	Severity	Location
Input Sanitization		GenericFactory.sol:L108-L112

Description:

The referenced code will permit an overwrite of the upgradeAdmin in the GenericFactory, the only member with administrative privileges, in a push pattern.

Impact:

A mistake during administrator adjustments would result in permanent forfeiture of the upgradeAdmin role which we consider an ill-advised trait.

Example:

Copy
108function setUpgradeAdmin(address newUpgradeAdmin) external nonReentrant adminOnly {
109    if (newUpgradeAdmin == address(0)) revert E_BadAddress();
110    upgradeAdmin = newUpgradeAdmin;
111    emit SetUpgradeAdmin(newUpgradeAdmin);
112}

Recommendation:

We advise a pull pattern to be adopted instead, proposing a new administrator which would then have to explicitly accept administrator-ship via a separate dedicated function.

Alleviation (fb2dd77a6ff9b7f710edb48e7eb5437e0db4fc1a):

The Euler Finance team evaluated this exhibit and clarified that they utilized a push-based ownership pattern in other instances of the codebase in addition to this one, and that they are comfortable with the push-based mechanism.

GFY-02M: Potentially Improper Exposure of Data

Type	Severity	Location
Standard Conformity		GenericFactory.sol:L22, L30, L118

Description:

The proxyLookup mapping is exposed publicly yet will perform mutations when queried via its dedicated getter function due to a potentially outdated data point.

Impact:

Direct invocations of the GenericFactory::proxyLookup compiler-generated getter function may yield outdated data points that callers may not be necessarily aware of.

Example:

Copy
116function getProxyConfig(address proxy) external view returns (ProxyConfig memory config) {
117    config = proxyLookup[proxy];
118    if (config.upgradeable) config.implementation = implementation;
119}

Recommendation:

We advise the mapping to not be accessible directly, as it should always be queried via the GenericFactory::getProxyConfig function.

Alleviation (fb2dd77a6ff9b7f710edb48e7eb5437e0db4fc1a):

The proxyLookup member has been set as internal per our recommendation, ensuring that the data point is correctly fetched via the GenericFactory::getProxyConfig function at all times.