Summary

The current protocol proxy system does not work as a proxy because it is impossible to update the logic contract via its proxy. The logic contract can be updated directly from the factory contract, which violates the proxy idea.

Vulnerability Details

When deploying the Factory contract - MembershipFactory - a new proxyAdmin contract is created in the constructor:

Here the msg.sender will be the new admin for proxyAdmin contract.

When we create a new DAO we use current proxyAdmin contract for a proxyAdmin of Membership contract:

However it is not possible to update Logic contract by calling proxyAdmin from another ProxyAdmin contract.

In the same time there is a separate function in Factory contract to update Logic:

Although the system works well for the DAO creation process, it is not the way proxy should work.

In transparent proxy system all the information from the current Logic contract is stored in the Proxy contract, so updating Logic from Factory works just like deploying a new Proxy with new Logic, but does not update it.

Impact

Excess gas for the system that is not working as it should.

Tools Used

Manual review

Recommendations

In my humble opinion, it's better to get rid of the proxy system if it's not working the way it's supposed to, to save gas and reduse deployment costs. Or provide the correct msg.sender to both ProxyAdmin contracts so they can update the Logic contract at any time.