QA Report

[code423n4]

Low - Proxy inheritance not initialized

Impact

There is no call of the initialize() function in the inherited OwnableProxyDelegation.sol contract in NestedFactory.sol. It is possible the intention is to perform this call manually, which is why I labeled this only a medium, but there could be problems if the proxy is not initialized at all. Because the code does not call the initialize(), I must assume the latter.

Proof of Concept

When a contract inherits another contract, the child contract calls the constructor of the parent contract. In this case, the OwnableProxyDelegation.sol contract has no constructor. The initialize function can only be called once, but this function sets the proxy owner, a critically important role for managing the system. If this role is not set when the contracts as first initialized, it is possible an owner of address(0) could cause problems. Luckily there is a require statement in line 26 which prevents arbitrary calls from any user to the initialize function.

Uninitialized proxies have been the cause of some large issues recently. See example A and example B.

Recommended Mitigation Steps

Add the following line to the NestedFinance.sol constructor:

initialize(_msgSender());

[maximebrugel]

As mentioned:

"Luckily there is a require statement in line 26 which prevents arbitrary calls from any user to the initialize function."

Also, if the owner of the proxy doesn't call the initialize function via upgradeToAndCall, he can't set the FeeSplitter address and all users calls will fail.

[harleythedogC4]

Agree with sponsor with the reasoning why this is invalid.