Project

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

It is possible to join public or private DAO and ignore noOfTiers

Summary

SPONSORED DAOs have a fixed number of noOfTiers = 6. However, Private and Public DAOs can have noOfTiers from 1 to 6.

When joining a DAO, a check is made in joinDao that tierIndex is not greater than noOfTiers. Note that there is no check for this directly in the MembershipERC1155::mint function that is called to get the NFT.

// MEMBERSHIP Factory contract
function joinDAO(address daoMembershipAddress, uint256 tierIndex) external {
require(daos[daoMembershipAddress].noOfTiers > tierIndex, "Invalid tier.");
require(daos[daoMembershipAddress].tiers[tierIndex].amount > daos[daoMembershipAddress].tiers[tierIndex].minted, "Tier full.");
uint256 tierPrice = daos[daoMembershipAddress].tiers[tierIndex].price;
uint256 platformFees = (20 * tierPrice) / 100;
daos[daoMembershipAddress].tiers[tierIndex].minted += 1;
IERC20(daos[daoMembershipAddress].currency).transferFrom(_msgSender(), owpWallet, platformFees);
IERC20(daos[daoMembershipAddress].currency).transferFrom(_msgSender(), daoMembershipAddress, tierPrice - platformFees);
IMembershipERC1155(daoMembershipAddress).mint(_msgSender(), tierIndex, 1);
emit UserJoinedDAO(_msgSender(), daoMembershipAddress, tierIndex);
}
// MEMBERSHIPERC1155 contract
function mint(address to, uint256 tokenId, uint256 amount) external override onlyRole(OWP_FACTORY_ROLE) {
totalSupply += amount * 2 ** (6 - tokenId); // Update total supply with weight
_mint(to, tokenId, amount, "");
}

However, if you simply call MembershipERC1155::mint via callExternalContract with an arbitrary tierIndex from 0 to 6 - the function will successfully print the specified user his NFT, ignoring noOfTiers.

function callExternalContract(address contractAddress, bytes memory data) external payable onlyRole(EXTERNAL_CALLER) returns (bytes memory ) {
(bool success, bytes memory returndata) = contractAddress.call{value: msg.value}(data);
require(success, "External call failed");
return returndata;
}

Vulnerability Details

It breaks the protocol invariant that in PUBLIC and PRIVATE DAOs users can have any NFT with id from 0 to 6, not from 0 to numOfTiers as set at initialisation.

Impact

Incorrect allocation of shares may affect the distribution of awards.
Severity: Medium

Tools Used

Manual Review

Recommendations

Add check to mint function, that tokenId < numOfTiers

Updates

Lead Judging Commences

0xbrivan2 Lead Judge about 1 year ago
Submission Judgement Published
Invalidated
Reason: Incorrect statement
0xbrivan2 Lead Judge about 1 year ago
Submission Judgement Published
Invalidated
Reason: Incorrect statement

Support

FAQs

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

Give us feedback!