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

RegistryFactory should validate that there are no duplicate attesters

adriro

Summary

The presence of duplicate attesters could compromise the security validations when creating accounts through the RegistryFactory contract.

Vulnerability Details

The RegistryFactory contract is a factory for the Nexus smart account that works in conjunction with the ERC-7484 Registry. When creating an account, the factory executes some validations against the selected modules by checking if the configured attesters have vouched for the module using the registry.

These attesters are addresses that are stored in the contract using an array and can be added using the addAttester() function.

57: function addAttester(address attester) external onlyOwner {
58: attesters.push(attester);
59: }

Note here that there are no validations in place to check if a certain attester has been already configured in the contract, new values are simply pushed to the array.

The presence of duplicate attesters would result in the same attester being counted multiple times during the threshold check, undermining the security of the validations. This issue is evident in the reference implementation of the check() function in ERC-7484, where checks are performed individually on each array element.

function check(
address module,
address[] calldata attesters,
uint256 threshold
)
external
view
{
uint256 validCount = 0;
for (uint256 i = 0; i < attesters.length; i++) {
bool isValid = _check(module, attesters[i]);
if (isValid) validCount++;
}
if (validCount < threshold) revert AttestationThresholdNotMet();
}

Impact

Duplicate attesters could accidentally or intentionally weaken the security validations as the same attester could be counted more than once to reach the configured threshold.

Tools Used

None.

Recommendations

When adding an attester, validate if it is already present in the list.

function addAttester(address attester) external onlyOwner {
+ for (uint256 i = 0; i < attesters.length; i++) {
+ require(attesters[i] != attester);
+ }
attesters.push(attester);
}
Updates

Lead Judging Commences

0xnevi Lead Judge 11 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 10 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.