Biconomy: Nexus
Biconomy
HardhatFoundry
30,000 USDC
View results
Previous Next
Submission Details
Severity: medium
Invalid

Using 'continue' Instead of 'revert' in Iteration

juggernaut63

File location:

Summary

A vulnerability was discovered in the Solidity contract that uses the 'revert' command in iterations to validate modules. Using 'revert' could be exploited by malicious actors who might introduce array objects that fail condition checks, thereby aborting the entire transaction.

Vulnerability Details

This vulnerability exists in the 'RegistryFactory.sol' contract where the 'revert' command is used in an iteration to validate the module. When the condition is not met, 'revert' aborts the entire transaction, which can be exploited by malicious actors to disrupt group operations by introducing array objects that fail the condition check.

Impact

By using 'revert' in iteration, the entire transaction will be rolled back if one element in the array does not meet the condition. This allows attackers to disrupt group operations by introducing invalid elements in the array, which can result in unnecessary transaction failures and service disruptions.

Tools Used

  • Inspection manual

  • Solidity

  • Foundry

Recommendations

Replace the 'revert' command with 'continue' in iterations to skip array indices that do not satisfy the condition, unless there is a valid security or logical reason to use 'revert'. This ensures group operations continue without interruption.

Code snippet:

L98-L100

for (uint256 i = 0; i < validators.length; i++) {
require(isModuleAllowed(validators[i].module, MODULE_TYPE_VALIDATOR), ModuleNotWhitelisted(validators[i].module));
}

L102-L104

for (uint256 i = 0; i < executors.length; i++) {
require(isModuleAllowed(executors[i].module, MODULE_TYPE_EXECUTOR), ModuleNotWhitelisted(executors[i].module));
}

L108-L110

for (uint256 i = 0; i < fallbacks.length; i++) {
require(isModuleAllowed(fallbacks[i].module, MODULE_TYPE_FALLBACK), ModuleNotWhitelisted(fallbacks[i].module));
}

Fixed code:

for (uint256 i = 0; i < validators.length; i++) {
if (!isModuleAllowed(validators[i].module, MODULE_TYPE_VALIDATOR)) {
continue;
}
for (uint256 i = 0; i < executors.length; i++) {
if (!isModuleAllowed(executors[i].module, MODULE_TYPE_EXECUTOR)) {
continue;
}
​
require(isModuleAllowed(hook.module, MODULE_TYPE_HOOK), ModuleNotWhitelisted(hook.module));
for (uint256 i = 0; i < fallbacks.length; i++) {
if (!isModuleAllowed(fallbacks[i].module, MODULE_TYPE_FALLBACK)) {
continue;
}
}

Code when testing using Foundry:

contract RegistryFactory {
struct Module {
address module;
}
​
Module[] public validators;
Module[] public executors;
Module[] public fallbacks;
Module public hook;
​
uint8 public constant MODULE_TYPE_VALIDATOR = 1;
uint8 public constant MODULE_TYPE_EXECUTOR = 2;
uint8 public constant MODULE_TYPE_FALLBACK = 3;
uint8 public constant MODULE_TYPE_HOOK = 4;
​
constructor() {
setHook(address(0x1)); // Atur hook di konstruktor
}
​
function isModuleAllowed(address _module, uint8 _moduleType) public pure returns (bool) {
// Simulasi pengecekan apakah modul diizinkan
return _module != address(0);
}
​
function addValidator(address _module) public {
validators.push(Module(_module));
}
​
function addExecutor(address _module) public {
executors.push(Module(_module));
}
​
function setHook(address _module) public {
hook = Module(_module);
}
​
function addFallback(address _module) public {
fallbacks.push(Module(_module));
}
​
function testModules() public view {
for (uint256 i = 0; i < validators.length; i++) {
if (!isModuleAllowed(validators[i].module, MODULE_TYPE_VALIDATOR)) {
continue;
}
}
for (uint256 i = 0; i < executors.length; i++) {
if (!isModuleAllowed(executors[i].module, MODULE_TYPE_EXECUTOR)) {
continue;
}
}
​
require(isModuleAllowed(hook.module, MODULE_TYPE_HOOK), "ModuleNotWhitelisted: hook.module");
​
for (uint256 i = 0; i < fallbacks.length; i++) {
if (!isModuleAllowed(fallbacks[i].module, MODULE_TYPE_FALLBACK)) {
continue;
}
}
}
}

Fouundry output:

Ran 1 test for src/RegistryFactory.sol:RegistryFactory
[PASS] testModules() (gas: 8742)
Suite result: ok. 1 passed; 0 failed; 0 skipped; finished in 51.09ms (6.99ms CPU time)

Ran 4 tests for test/RegistryFactoryTest.sol:RegistryFactoryTest
[PASS] testExecutorModules() (gas: 81337)
[PASS] testFallbackModules() (gas: 81277)
[PASS] testHookModule() (gas: 15109)
[PASS] testValidatorModules() (gas: 81443)
Suite result: ok. 4 passed; 0 failed; 0 skipped; finished in 54.23ms (7.63ms CPU time)

Ran 2 test suites in 106.02ms (105.31ms CPU time): 5 tests passed, 0 failed, 0 skipped (5 total tests)

Updates

Lead Judging Commences

0xnevi Lead Judge
11 months ago
0xnevi Lead Judge 11 months ago
Submission Judgement Published
Invalidated
Reason: Design choice

Support

FAQs

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