Project

Summary

The createNewDAOMembership() function is supposed to create a new DAO membership by deploying a transparent proxy.

OpenZeppelin’s TransparentUpgradeableProxy contract expects the address of an initialOwner and automatically deploys its own ProxyAdmin. However, in the membershipFactory.sol code, a separate ProxyAdmin instance is deployed manually (let’s call it PA1), and its address is passed to the TransparentUpgradeableProxy as the initialOwner.

As a result, when TransparentUpgradeableProxy deploys its own ProxyAdmin (let’s call this one PA2), it sets PA1 as the owner of PA2. But PA1 is simply a deployed contract and can't interact with PA2 to perform upgrades. This configuration blocks any upgrades for the proxy.

The flow looks like this:

• msg.sender → proxyAdmin1 (PA1) → proxyAdmin2 (PA2) → TransparentProxy

Vulnerability Details

1. InmembershipFactory.solthe constructor deploys a proxyAdmin (lets call it PA1 contract):

   proxyAdmin = new ProxyAdmin(msg.sender);

2. user calls createNewDAOMembership()function to create a new DAO membership

3. This function creates a TransparentUpgradeableProxy and passes PA1 address as initialOwner:

   TransparentUpgradeableProxy proxy = new TransparentUpgradeableProxy(

   membershipImplementation,

   address(proxyAdmin), //PA1

   abi.encodeWithSignature("initialize(string,string,string,address,address)", daoConfig.ensname, "OWP", baseURI, _msgSender(), daoConfig.currency)

   );

4. Inside the TransparentUpgradeableProxy constructor, another ProxyAdmin (let's call it PA2) is deployed, with PA1 set as its initialOwner. This code in OpenZeppelin’s library demonstrates that:

   https://github.com/OpenZeppelin/openzeppelin-contracts/blob/448efeea6640bbbc09373f03fbc9c88e280147ba/contracts/proxy/transparent/TransparentUpgradeableProxy.sol#L80C1-L80C56

   constructor(address _logic, address initialOwner, bytes memory _data) payable ERC1967Proxy(_logic, _data) {

   _admin = address(new ProxyAdmin(initialOwner));

   // Set the storage value and emit an event for ERC-1967 compatibility

   ERC1967Utils.changeAdmin(_proxyAdmin());

   }

5. Consequently, PA1 (deployed contract) is the owner of PA2. And PA2 acts as the proxyAdmin of the proxy. PA1 can not call functions on PA2 to perform upgrades or change admin. As a result, the proxy cannot be upgraded

Impact

The membership DAO cannot be upgraded due to an incorrect configuration of the proxyAdmin ownership.

Tools Used

vscode, manual review

Recommendations

don't deploy a proxyAdmin and just set msg.sender as owner of proxyAdmin (which will be deployed by openZeplin library automatically):