code-423n4 / 2023-05-maia-findings

24 stars 13 forks source link

Incorrect variable use in the `addBridgeAgentFactory` function #574

Closed code423n4 closed 1 year ago

code423n4 commented 1 year ago

Lines of code

https://github.com/code-423n4/2023-05-maia/blob/54a45beb1428d85999da3f721f923cbf36ee3d35/src/ulysses-omnichain/RootPort.sol#L361-L418

Vulnerability details

Impact

The incorrect usage of a variable in the addBridgeAgentFactory function can potentially lead to inconsistent data tracking, making it more difficult to accurately track the number of Bridge Agent Factories. This may impact the integrity of the system's state, and potentially expose other vulnerabilities, impacting the overall security of the contract.

Proof of Concept

The addBridgeAgentFactory function in the contract is meant to add new Bridge Agent Factories to the system and increment the counter tracking the total number of factories. However, the function erroneously uses bridgeAgentsLenght instead of bridgeAgentFactoriesLenght to increment the count of Bridge Agent Factories.

This can cause inconsistent and inaccurate tracking of the number of Bridge Agent Factories and Bridge Agents. As a result, there can be a discrepancy between the actual number of Bridge Agent Factories and the number reported by the bridgeAgentFactoriesLenght variable.

Here's the problematic code snippet:


function addBridgeAgentFactory(address _bridgeAgentFactory) external onlyOwner {
    bridgeAgentFactories[bridgeAgentsLenght++] = _bridgeAgentFactory;
    emit BridgeAgentFactoryAdded(_bridgeAgentFactory);
}

Tools Used

Manual review

Recommended Mitigation Steps

Replace the variable bridgeAgentsLenght with bridgeAgentFactoriesLenght in the addBridgeAgentFactory function. This will ensure that the correct variable is incremented when a new Bridge Agent Factory is added, and thus accurately track the total number of Bridge Agent Factories. The corrected code should look like this:


function addBridgeAgentFactory(address _bridgeAgentFactory) external onlyOwner {
-    bridgeAgentFactories[bridgeAgentsLenght++] = 
+    bridgeAgentFactories[bridgeAgentFactoriesLenght++] =     
_bridgeAgentFactory;
    emit BridgeAgentFactoryAdded(_bridgeAgentFactory);
}

Assessed type

Other

c4-judge commented 1 year ago

trust1995 marked the issue as primary issue

c4-judge commented 1 year ago

trust1995 marked the issue as satisfactory

c4-judge commented 1 year ago

trust1995 marked the issue as duplicate of #372