DeFiFoundry
60,000 USDC
View results
Submission Details
Severity: low
Invalid

no check to verify if `initializables.length` is equal to `initializePayloads.length` in branch upgrade function

Summary

no check to verify if initializables.length is equal to initializePayloads.length in branch upgrade function

Vulnerability Details

function initializeRootUpgrade(
RootProxy.BranchUpgrade[] memory,
@> address[] memory initializables,
@> bytes[] memory initializePayloads
)
internal
{
@> for (uint256 i; i < initializables.length; i++) { // @audit-issue didn't check if initializables.length == initializePayloads.length
address initializable = initializables[i];
bytes memory data = initializePayloads[i];
if (initializable.code.length == 0) {
revert Errors.InitializableIsNotContract(initializable);
}
Address.functionDelegateCall(initializable, data);
}
}
}

From the above code, we can see that there is no check to verify if initializables.length is equal to initializePayloads.length.
UpgradeBranch::upgrade call RootUpgrade::upgrade. RootUpgrade::upgrade call RootUpgrade::initializeRootUpgrade.
None of these three functions (RootUpgrade::initializeRootUpgrade, RootUpgrade::upgrade and UpgradeBranch::upgrade) perform this check.

Impact

Due to the lack of a check to ensure that initializables.length is equal to initializePayloads.length, a mismatch in the lengths of the two arrays during the for loop can occur. This maybe lead to a delegatecall to an address with a data input of empty.

Tools Used

manual

Recommendations

Add checks if initializables.length is equal to initializePayloads.length.

function initializeRootUpgrade(
RootProxy.BranchUpgrade[] memory,
address[] memory initializables,
bytes[] memory initializePayloads
)
internal
{
+ require(initializables.length == initializePayloads.length);
for (uint256 i; i < initializables.length; i++) { // @audit-issue didn't check if initializables.length == initializePayloads.length
address initializable = initializables[i];
bytes memory data = initializePayloads[i];
if (initializable.code.length == 0) {
revert Errors.InitializableIsNotContract(initializable);
}
Address.functionDelegateCall(initializable, data);
}
}
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!