ArkProject: NFT Bridge
ArkProject
NFTBridge
60,000 USDC
View results
Previous Next
Submission Details
Severity: low
Invalid

Failure to Upgrade Bridge Contract with a New Owner

bladesec

Summary

When attempting to upgrade the bridge contract, the initialize function is invoked to set up the new configuration. However, due to a flaw in how the initialize function is implemented, the process will fail if a new owner is specified during the upgrade.

Vulnerability Details

Below is the code snippet for the initialize function:

function initialize(
bytes calldata data
)
public
onlyInit
{
(
address owner,
IStarknetMessaging starknetCoreAddress,
uint256 starklaneL2Address,
uint256 starklaneL2Selector
) = abi.decode(
data,
(address, IStarknetMessaging, uint256, uint256)
);
_enabled = false;
_starknetCoreAddress = starknetCoreAddress;
​
_transferOwnership(owner);
​
setStarklaneL2Address(starklaneL2Address);
setStarklaneL2Selector(starklaneL2Selector);
}

The function begins by transferring ownership to the new owner specified in the data parameter. It then proceeds to set the Starklane L2 address and selector using setStarklaneL2Address and setStarklaneL2Selector functions.

However, the problem arises because both setStarklaneL2Address and setStarklaneL2Selector are functions that can only be executed by the admin. Since the ownership is transferred to the new owner before these functions are called, the original owner no longer has the necessary permissions to complete the process. As a result, the upgrade will fail if a new owner address is provided.

Impact

This vulnerability prevents the current owner from successfully upgrading the bridge contract when attempting to assign a new owner during the upgrade process.

Tools Used

Manual Review

Recommendations

To resolve this issue, the ownership transfer should be moved to the end of the initialize function. This ensures that the original owner retains the necessary permissions to execute all required admin functions before transferring ownership.

Here is the modified code:

function initialize(
bytes calldata data
)
public
onlyInit
{
(
address owner,
IStarknetMessaging starknetCoreAddress,
uint256 starklaneL2Address,
uint256 starklaneL2Selector
) = abi.decode(
data,
(address, IStarknetMessaging, uint256, uint256)
);
_enabled = false;
_starknetCoreAddress = starknetCoreAddress;
​
- _transferOwnership(owner);
​
setStarklaneL2Address(starklaneL2Address);
setStarklaneL2Selector(starklaneL2Selector);
​
+ _transferOwnership(owner);
}
Updates

Lead Judging Commences

n0kto Lead Judge 12 months ago
Submission Judgement Published
Invalidated
Reason: Incorrect statement
Assigned finding tags:

Informational / Gas

Please, do not suppose impacts, think about the real impact of the bug and check the CodeHawks documentation to confirm: https://docs.codehawks.com/hawks-auditors/how-to-determine-a-finding-validity A PoC always helps to understand the real impact possible.

Support

FAQs

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