Zaros Part 1
Zaros
DeFiFoundry
60,000 USDC
View results
Previous Next
Submission Details
Severity: low
Invalid

L-01. There is no check in `UpgradeBranch::upgrade` that the lengths of the `initializables` and `initializePayloads ` arrays are the same that could result in an incorrect configuration of the protocol

stacey

https://github.com/Cyfrin/2024-07-zaros/blob/d687fe96bb7ace8652778797052a38763fbcbb1b/src/tree-proxy/branches/UpgradeBranch.sol#L19-L30

Summary

The protocol contains a issue within the UpgradeBranch::upgrade function where there is no validation ensuring that the lengths of the initializables and initializePayloads arrays match. This oversight could lead to an improperly configured protocol if the length of initializables is less than that of initializePayloads.

Vulnerability Details

​
FILE: src/tree-proxy/branches/UpgradeBranch.sol
​
function upgrade(
RootProxy.BranchUpgrade[] memory branchUpgrades,
address[] memory initializables,
bytes[] memory initializePayloads
)
external
{
_authorizeUpgrade(branchUpgrades);
RootUpgrade.Data storage rootUpgrade = RootUpgrade.load();
​
rootUpgrade.upgrade(branchUpgrades, initializables, initializePayloads);
}
​

RootUpgrade::initializeRootUpgrade function is internal function that provides the logic for initialization.

​
FILE: src/tree-proxy/leaves/RootUpgrade.sol
​
function initializeRootUpgrade(
RootProxy.BranchUpgrade[] memory,
address[] memory initializables,
bytes[] memory initializePayloads
)
internal
{
for (uint256 i; i < initializables.length; i++) {
address initializable = initializables[i];
bytes memory data = initializePayloads[i];
​
if (initializable.code.length == 0) {
revert Errors.InitializableIsNotContract(initializable);
}
​
Address.functionDelegateCall(initializable, data);
}
}
​

If initializables.length < initializePayloads.length transaction doesn't revert. This implies that the initialization process will continue despite lacking sufficient data for complete initialization of all branches. Consequently, some implementations intended to be initialized will remain uninitialized. This could result in an incorrect configuration of the protocol, as expected functions or parameters may not be properly set up. Such circumstances can introduce vulnerabilities in the protocol, as incomplete initialization might disrupt contract logic or allow malicious actors to exploit code gaps to achieve undesirable outcomes.

Impact

The primary impact of this vulnerability is the potential for an incorrectly configured protocol, which could compromise its security and functionality. Given that the transaction does not revert due to the absence of a check for matching array lengths, such misconfiguration may be difficult to detect.

Tools Used

Manual Review

Recommended Mitigation

Add the require in the UpgradeBranch::upgrade function to check that lengths of the initializables and initializePayloads arrays are equal.

​
function upgrade(
RootProxy.BranchUpgrade[] memory branchUpgrades,
address[] memory initializables,
bytes[] memory initializePayloads
)
external
{
_authorizeUpgrade(branchUpgrades);
+ require (initializables.length == initializePayloads.lendth, "Incorrect arrays length");
RootUpgrade.Data storage rootUpgrade = RootUpgrade.load();
​
rootUpgrade.upgrade(branchUpgrades, initializables, initializePayloads);
}
​
Updates

Lead Judging Commences

inallhonesty Lead Judge
over 1 year ago
inallhonesty Lead Judge over 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!