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

ERC1155Upgradeable has it's own initialization logic but protocol fails to implement it in MembershipERC1155

inallhonesty

Summary

Circumventing the __ERC1155_initexposes every instance of the MembershipERC1155 to state corruption / faulty URI functionality in case of Upgrade.

Vulnerability Details

As we can see below the initializefunction of MembershipERC1155doesn't properly initialize the OZ upgradable contracts inherited.

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);
}

While [Low-4] Contract doesn't initialize inherited OZ Upgradeable contractsof LightChaser recognizes the problem for AccessControlUpgradeablethe same problem persists in case of ERC1155Upgradeablewhich also has it's own initialization path:

function __ERC1155_init(string memory uri_) internal onlyInitializing {
__ERC1155_init_unchained(uri_);
}
​
function __ERC1155_init_unchained(string memory uri_) internal onlyInitializing {
_setURI(uri_);
}

Impact

The overall severity is low.

  • The likelihood of OZ changing ERC1155Upgradeableto include multiple variables in their initialization chain is low.

  • But if they change their own function to affect more than one variable (currently the uri_) then the upgradability of all MembershipERC1155will be severely impacted, thus I consider the impact medium.

Tools Used

Endless imagination

Recommendations

Perform the following changes:

function initialize(
string memory name_,
string memory symbol_,
string memory uri_,
address creator_,
address currency_
) external initializer {
// Proper parent initialization ensuring all state variables
// are set up correctly and future-proof
++ __ERC1155_init(uri_); //known
++ __AccessControl_init(); //not known
// Our own initialization
_name = name_;
_symbol = symbol_;
creator = creator_;
currency = currency_;
-- _setURI(uri_); //eliminated because it's properly done through ERC1155 init
// Additional role setup
_grantRole(DEFAULT_ADMIN_ROLE, msg.sender);
_grantRole(DAO_CREATOR, creator_);
_grantRole(OWP_FACTORY_ROLE, msg.sender);
}
Updates

Lead Judging Commences

0xbrivan2 Lead Judge about 1 year ago
Submission Judgement Published
Invalidated
Reason: Non-acceptable severity

Support

FAQs

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

Give us feedback!