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

Letting only the `DAO_CREATOR` role to call `setURI` method in `MembershipERC1155` contract can mess up the DAO's token URI

hieutrinh02

Summary

According to the design, when user creates new DAO Membership through createNewDAOMembership, a new MembershipERC1155 contract is initialized with the URI set to the baseURI variable value in MembershipFactory contract. This value can be modified later through the setURI method in MembershipERC1155 contract. However, this method allows only the DAO_CREATOR role, which is originally the DAO's creator, to update the base URI for all token types. This design introduces a potential risk where the DAO creator may accidentally or maliciously modify the URI, thereby disrupting access to the token metadata. Moreover, without an override mechanism, other roles, such as OWP_FACTORY_ROLE, DEFAULT_ADMIN_ROLE, cannot intervene to correct any misconfiguration, posing risks to the integrity and availability of token metadata.

Vulnerability Details

The setURI method in MembershipERC1155 contract is restricted to being called by only the DAO_CREATOR role, originally the DAO's creator. This function updates the base URI for all tokens by invoking the _setURI(newURI) method

https://github.com/Cyfrin/2024-11-one-world/blob/main/contracts/dao/tokens/MembershipERC1155.sol#L105-L109

/// @notice Set a new URI for all token types
/// @param newURI The new URI to set
@> function setURI(string memory newURI) external onlyRole(DAO_CREATOR) {
_setURI(newURI);
}

The DAO_CREATOR role is granted only to the DAO's creator when he/she creates the DAO

https://github.com/Cyfrin/2024-11-one-world/blob/main/contracts/dao/tokens/MembershipERC1155.sol#L39-L54

function initialize(
string memory name_,
string memory symbol_,
string memory uri_,
address creator_,
address currency_
) external initializer {
_name = name_;
_symbol = symbol_;
creator = creator_;
currency = currency_;
_setURI(uri_);
_grantRole(DEFAULT_ADMIN_ROLE, msg.sender);
@> _grantRole(DAO_CREATOR, creator_);
_grantRole(OWP_FACTORY_ROLE, msg.sender);
}

Also in the above initialize function, the uri_ parameter is set to the URI, which is originally the baseURI variable value in MembershipFactory contract

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

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

This baseURI will then be used to retrieve the complete URI with the format of: baseURI + membershipAddress + "/" + tokenId

https://github.com/Cyfrin/2024-11-one-world/blob/main/contracts/dao/tokens/MembershipERC1155.sol#L117-L124

function uri(uint256 tokenId) public view virtual override returns (string memory) {
return string(abi.encodePacked(
super.uri(tokenId),
Strings.toHexString(uint256(uint160(address(this))), 20),
"/",
Strings.toString(tokenId)
));
}

If the DAO's creator accidentally or maliciously modify the URI that's invalid or not follow the format above, access to the MembershipERC1155 token metadata will be broken.

Impact

The vulnerability poses the following risks to the DAO:

  • Loss of Token Metadata Access: If the DAO's creator sets an incorrect or malformed URI, all token types may point to inaccessible or invalid metadata, which can render tokens effectively unusable within the ecosystem.

  • No Recovery Mechanism: The absence of override authority (e.g., intervention by OWP_FACTORY_ROLE role or another privileged role) means the DAO is at risk of permanent disruption if the DAO's creator acts incorrectly or becomes inactive.

  • Centralization Risk: Granting the sole power to manage URIs to one role may contradict the decentralized ethos of a DAO and lead to trust and governance issues, especially if misuse occurs.

Tools Used

Manual Review

Recommendations

  • Grant the DAO_CREATOR role to also the MembershipFactory address or another admin address to call setURI if needed.

function initialize(
string memory name_,
string memory symbol_,
string memory uri_,
address creator_,
address currency_
) external initializer {
_name = name_;
_symbol = symbol_;
creator = creator_;
currency = currency_;
_setURI(uri_);
_grantRole(DEFAULT_ADMIN_ROLE, msg.sender);
_grantRole(DAO_CREATOR, creator_);
+ _grantRole(DAO_CREATOR, msg.sender);
_grantRole(OWP_FACTORY_ROLE, msg.sender);
}

  • Consider implementing a timelock mechanism for URI changes, giving other members of the DAO time to review and potentially veto changes. This prevents immediate execution of potentially harmful modifications.

Updates

Lead Judging Commences

0xbrivan2 Lead Judge 7 months ago
Submission Judgement Published
Invalidated
Reason: Design choice
0xbrivan2 Lead Judge 7 months ago
Submission Judgement Published
Invalidated
Reason: Design choice

Support

FAQs

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