Biconomy: Nexus
Biconomy
HardhatFoundry
30,000 USDC
View results
Previous Next
Submission Details
Severity: low
Valid

`RegistryFactory` is not fully comliant with ERC7484 as `RegistryFactory::addAttester` allows duplicate attesters to be added.

sheep

Description:

According to ERC-7484, for check functions The attesters provided MUST be unique and sorted and the Registry MUST revert if they are not. RegistryFactory::addAttester function lets us add the attester to the RegistryFactory::attesters array which are passed in Registry::isModuleAllowed to the REGISTRY::check function. Registry::isModuleAllowed function is being used in Registry::createAccount function, which uses it to check if the Module being passed to create the account instance is allowed or not, otherwise it will revert. So if there are duplicates in the attesters array the REGISTRY::check function will revert and user will be unable to create acount through ResgistryFactory.

link to the eip https://eips.ethereum.org/EIPS/eip-7484

Impact:

Adding suplicate attesters can render users unable to create account instances.

Proof of Concept:

function test_DuplicateAttestersCanBeAdded() public {
address attester = address(0x456);
address attester2 = address(0x456);
vm.startPrank(FACTORY_OWNER.addr);
registryFactory.addAttester(attester);
registryFactory.addAttester(attester2);
assert(address(attester) == address(attester2));
assertTrue(registryFactory.attesters(1) == attester, "Attester should be added");
assertTrue(registryFactory.attesters(2) == attester2, "Attester should be added");
​
}

Recommended Mitigation:

  1. Use a mapping to keep the track of attesters index in the array mapping(address => uint256) use this while adding attester and associate its address to index at which it is being added. check this mapping when additional enteries so that there are no duplicates.

  2. Loop through the array to check that attester being added, does not matches the existing enteries.

Updates

Lead Judging Commences

0xnevi Lead Judge 12 months ago
Submission Judgement Published
Invalidated
Reason: Other
Assigned finding tags:

finding-ERC7848-add-duplicate-attester

Invalid, - Addition of attesters are admin only functionalities so if duplicate addresses are added it would consitute admin input/call validation. - ERC-7484 is in draft mode so we should not take it as the final EIP configuration yet. - Even if a mistake was made, removals can be performed by invoking `removeAttester` multiple times by the owner to completely remove a duplicate user. - In the [documentation](https://github.com/bcnmy/nexus/wiki#problems-nexus-solves), it is not noted that Nexus suite will be ERC7484 compliant.

Appeal created

0xnevi Lead Judge 11 months ago
Submission Judgement Published
Validated
Assigned finding tags:

finding-ERC7484-sorted-duplicate-attestor-issue

Invalid, similar issue to #151 and duplicates - Addition of attesters are admin only functionalities so if duplicate addresses are added it would consitute admin input/call validation. - ERC-7484 is in draft mode so we should not take it as the final EIP configuration yet. - In the [documentation](https://github.com/bcnmy/nexus/wiki#problems-nexus-solves), it is not noted that Nexus suite will be ERC7484 compliant.

Support

FAQs

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