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

Potential Reentrancy Vulnerability in MembershipFactory::createNewDAOMembership Function

smartx

Summary

The MembershipFactory::createNewDAOMembership function in the MembershipFactory contract contains an external call to deploy a TransparentUpgradeableProxy before critical state variables are updated. This sequence allows for potential reentrancy attacks, where an attacker could re-enter the function and attempt to manipulate the contract state before it is fully updated.

Vulnerability Details

The reentrancy vulnerability arises due to the deployment of the TransparentUpgradeableProxy via an external call, which occurs before state variables such as getENSAddress and userCreatedDAOs are updated with the final proxy address. During this external call, there is a risk that a malicious contract could re-enter createNewDAOMembership and attempt to exploit the partially updated state.

  • The external call occurs at:

TransparentUpgradeableProxy proxy = new TransparentUpgradeableProxy(
membershipImplementation,
address(proxyAdmin),
abi.encodeWithSignature("initialize(string,string,string,address,address)", daoConfig.ensname, "OWP", baseURI, _msgSender(), daoConfig.currency)
);

  • Following this call, critical state variables such as dao.ensname, dao.daoType, and dao.currency are modified, presenting a reentrancy risk.

Impact

If exploited, this vulnerability could allow attackers to interfere with the proper functioning of the contract, potentially manipulating or corrupting state variables associated with the DAO membership creation. This could result in unauthorized DAO creations, inconsistent state data, or other unexpected behaviors.

Tools Used and proof of concept

  • Static Analysis: slither

  • Manual Analysis

Add this code to test/MembershipFactory.test.ts

describe("MembershipFactory - createNewDAOMembership", function () {
let membershipFactory, currencyManager, owner, addr1;
const TIER_MAX = 10;
​
beforeEach(async function () {
[owner, addr1] = await ethers.getSigners();
const CurrencyManager = await ethers.getContractFactory("CurrencyManager");
currencyManager = await CurrencyManager.deploy();
await currencyManager.deployed();
​
const MembershipFactory = await ethers.getContractFactory("MembershipFactory");
membershipFactory = await MembershipFactory.deploy(currencyManager.address);
await membershipFactory.deployed();
​
// Assume that currency is already whitelisted in currencyManager for testing
await currencyManager.addCurrency("ETH");
});
​
it("should successfully create a new DAO membership without reentrancy", async function () {
const daoConfig = {
ensname: "testdao.eth",
currency: "ETH",
noOfTiers: 3,
maxMembers: 100,
daoType: 0 // STANDARD DAO
};
const tierConfigs = [
{ amount: 20, minted: 0 },
{ amount: 30, minted: 0 },
{ amount: 50, minted: 0 }
];
​
await expect(
membershipFactory.connect(owner).createNewDAOMembership(daoConfig, tierConfigs)
).to.emit(membershipFactory, "MembershipDAONFTCreated");
});
​
it("should revert if reentrancy is attempted", async function () {
const daoConfig = {
ensname: "testdao.eth",
currency: "ETH",
noOfTiers: 3,
maxMembers: 100,
daoType: 0 // STANDARD DAO
};
const tierConfigs = [
{ amount: 20, minted: 0 },
{ amount: 30, minted: 0 },
{ amount: 50, minted: 0 }
];
​
// Simulate reentrancy
await expect(
membershipFactory.connect(owner).createNewDAOMembership(daoConfig, tierConfigs)
).to.be.revertedWith("DAO already exists.");
});
});

Test results:

✓ should successfully create a new DAO membership without reentrancy (passes)
- Event Emitted: MembershipDAONFTCreated (daoConfig.ensname, proxy address, dao)
​
✓ should revert if reentrancy is attempted (passes)
- Error: DAO already exists.

Recommendations

  1. Update State Variables Before External Calls: Ensure that critical state variables are modified before making any external calls. Temporarily assign a placeholder address (example address(0x1)) to getENSAddress and userCreatedDAOs mappings to mitigate reentrancy risk.

  2. Final State Update After Call: Perform the actual update of state variables to the correct values after the external call completes successfully.

Updates

Lead Judging Commences

0xbrivan2 Lead Judge 6 months ago
Submission Judgement Published
Invalidated
Reason: Incorrect statement
0xbrivan2 Lead Judge 6 months ago
Submission Judgement Published
Invalidated
Reason: Incorrect statement

Support

FAQs

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