Summary
The MembershipERC1155 TransparentUpgradeableProxy is initialized with the wrong initialOwner address in the createNewDAOMembership function. This error occurs due to the assignment of proxyAdmin (an address of a contract) as the initialOwner, instead of an EOA (Externally Owned Account) that could perform future upgrades. Consequently, if upgrades are needed for the MembershipERC1155, they would be inaccessible since the proxyAdmin contract cannot invoke upgradeAndCall.
Vulnerability Details
The TransparentUpgradeableProxy contract requires three parameters: _logic, initialOwner, and _data. In createNewDAOMembership, the initialOwner is set as the proxyAdmin, this is the address of the proxyAdmin contract created in the MembershipFactory constructor. The TransparentUpgradeableProxy contract creates a new proxyAdmin instance and sets the originally created proxyAdmin (from MembershipFactory) as its owner.
The onlyOwner modifier on upgradeAndCall restricts future upgrades, as the proxyAdmin contract (incorrectly set as the initialOwner) cannot be used to invoke upgradeAndCall.
constructor(
address _currencyManager,
address _owpWallet,
string memory _baseURI,
address _membershipImplementation
) {
currencyManager = ICurrencyManager(_currencyManager);
baseURI = _baseURI;
owpWallet = _owpWallet;
membershipImplementation = _membershipImplementation;
@> proxyAdmin = new ProxyAdmin(msg.sender);
_grantRole(DEFAULT_ADMIN_ROLE, msg.sender);
_grantRole(EXTERNAL_CALLER, msg.sender);
}
function createNewDAOMembership(DAOInputConfig calldata daoConfig, TierConfig[] calldata tierConfigs)
external
returns (address)
{
require(currencyManager.isCurrencyWhitelisted(daoConfig.currency), "Currency not accepted.");
require(daoConfig.noOfTiers == tierConfigs.length, "Invalid tier input.");
require(daoConfig.noOfTiers > 0 && daoConfig.noOfTiers <= TIER_MAX, "Invalid tier count.");
require(getENSAddress[daoConfig.ensname] == address(0), "DAO already exist.");
if (daoConfig.daoType == DAOType.SPONSORED) {
require(daoConfig.noOfTiers == TIER_MAX, "Invalid tier count for sponsored.");
}
uint256 totalMembers = 0;
for (uint256 i = 0; i < tierConfigs.length; i++) {
totalMembers += tierConfigs[i].amount;
}
require(totalMembers <= daoConfig.maxMembers, "Sum of tier amounts exceeds maxMembers.");
console.log("admin1", address(proxyAdmin));
TransparentUpgradeableProxy proxy = new TransparentUpgradeableProxy(
membershipImplementation,
@> address(proxyAdmin),
abi.encodeWithSignature(
"initialize(string,string,string,address,address)",
daoConfig.ensname,
"OWP",
baseURI,
_msgSender(),
daoConfig.currency
)
);
DAOConfig storage dao = daos[address(proxy)];
dao.ensname = daoConfig.ensname;
dao.daoType = daoConfig.daoType;
dao.currency = daoConfig.currency;
dao.maxMembers = daoConfig.maxMembers;
dao.noOfTiers = daoConfig.noOfTiers;
for (uint256 i = 0; i < tierConfigs.length; i++) {
require(tierConfigs[i].minted == 0, "Invalid tier config");
dao.tiers.push(tierConfigs[i]);
}
getENSAddress[daoConfig.ensname] = address(proxy);
userCreatedDAOs[_msgSender()][daoConfig.ensname] = address(proxy);
emit MembershipDAONFTCreated(daoConfig.ensname, address(proxy), dao);
return address(proxy);
}
constructor(address _logic, address initialOwner, bytes memory _data) payable ERC1967Proxy(_logic, _data) {
@> _admin = address(new ProxyAdmin(initialOwner));
ERC1967Utils.changeAdmin(_proxyAdmin());
}
Impact
There is no way to upgrade the MembershipERC1155 proxy contract if necessary. This could result in functional and security issues remaining unresolved.
Tools Used
Manual Review
Recommendations
Update the createNewDAOMembership function to set initialOwner to an address capable of performing upgrades, such as an EOA that is the actual owner or DAO multisig address for more secure access management.
