Project
One World
NFTDeFi
15,000 USDC
View results
Previous Next
Submission Details
Severity: high
Valid

Nested proxyAdmin will lead upgrading failure of TransparentUpgradeableProxy

valy001

Summary

Nested proxyAdmin for TransparentUpgradebleProxy will make the Proxy of DaoMembership contract unable to ugprade.

Vulnerability Details

The DaoMembership is created by Factory contract through a TransparentUpgradeableProxy pattern. The transparent proxy mangage the admin role via the ProxyAdmin contract, which is created during the TransparentUpgradeableProxy constructor process. This will ease the procedure to upgrade implemention of the proxy. The ProxyAdmin::upgradeAndCall is supposed to call when upgrading the implementation, where in this call will be redirected to TransparentProxy. In such case, the admin of transparent proxy was set to be the AdminProxy. Howerver, the factory pass a already deployed proxyAdmin parameter to the TransparentUpgradeableProxy during createNewDAOMembership, which will lead the passed proxyAdmin as initalOwner to create another proxyAdmin. In this way the factory owner can never upgrade DAO proxy through the proxyAdmin created by owner in factory constructor process.

Essentially, this valnerability is caused by redundant proxyAdmin creating.

https://github.com/Cyfrin/2024-11-one-world/blob/1e872c7ab393c380010a507398d4b4caca1ae32b/contracts/dao/MembershipFactory.sol#L72-L76

POC

add the following code to MembershipFactory.test.ts under descibre("MembershipFactory", function(){}):

describe("Update implementation of DAO Proxy", function () {
it("Should fail to upgrade TransparentUpgradebleProxy", async function() {
const proxyAdminAddress =await membershipFactory.proxyAdmin();
console.log("proxyAdmin of DAOFactory", proxyAdminAddress);
await currencyManager.addCurrency(testERC20.address); // Assume addCurrency function exists in CurrencyManager
const tx = await membershipFactory.createNewDAOMembership(DAOConfig, TierConfig);
//console.log("TransparentProxy address : ", tx);
const receipt = await tx.wait();
const event2 = receipt.events.find((event:any) => event.event === "MembershipDAONFTCreated");
const ensName = event2.args[2][0];
const nftAddress = event2.args[1];
console.log("Transparent Proxy address : ", nftAddress)a;
const MembershipERC1155_2 = await ethers.getContractFactory('MembershipERC1155');
const membershipImplementation_2 = await MembershipERC1155_2.deploy();
await membershipImplementation_2.deployed();
​
console.log("implementation address old and new: ",
membershipImplementation.address, membershipImplementation_2.address);
const p_a = await ethers.getContractFactory("ProxyAdmin");
const proxyAdmin = p_a.attach(proxyAdminAddress);
const data = 0x00;
// const upgrade_imple = proxyAdmin.upgradeAndCall(nftAddress, membershipImplementation_2.address,data);
//@audit expect to be reverted : function selector was not recognized and there's no fallback function
await expect(proxyAdmin.upgradeAndCall(nftAddress, membershipImplementation_2.address,data)).to.be.reverted;
})
});

Impact

DaoMemership contract will not be able to upgrade in future time.

Tools Used

Manual

Recommendations

consider remove the ProxyAdmin pattern generation during MembershipFactory constructing process.

Updates

Lead Judging Commences

0xbrivan2 Lead Judge
9 months ago
0xbrivan2 Lead Judge 9 months ago
Submission Judgement Published
Validated
Assigned finding tags:

MembershipERC1155 implementations can not be upgraded for already deployed proxies

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.