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

`Starklane` Will Always Revert in `initialize` Unless `Owner` is `msg.sender`

Summary

The Starklane::initialize function is responsible for initializing the contract and setting the contract owner to the address provided in the input bytes data. However, the function also attempts to execute two privileged operations, setStarklaneL2Address and setStarklaneL2Selector, immediately after transferring ownership. If the owner parsed from data is not the same as msg.sender, these operations will fail, causing the entire initialize function to revert.

Vulnerability Details

In the Starklane::initialize function, the ownership of the contract is transferred to the owner address specified in the input data. This is done as follows:

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 key issue here is that after the _transferOwnership(owner) line, the contract expects the owner to execute the subsequent setStarklaneL2Address and setStarklaneL2Selector operations. However, if owner is not equal to msg.sender, these operations will fail because the caller has no authority at all.

The privileged functions setStarklaneL2Address and setStarklaneL2Selector are both protected by the onlyOwner modifier:

function setStarklaneL2Address(
uint256 l2Address
)
public
onlyOwner
{
_starklaneL2Address = Cairo.snaddressWrap(l2Address);
}
function setStarklaneL2Selector(
uint256 l2Selector
)
public
onlyOwner
{
_starklaneL2Selector = Cairo.felt252Wrap(l2Selector);
}

Since the ownership is transferred within the initialize function, any mismatch between msg.sender and the owner parsed from data will cause these function calls to fail, leading to a full reversion of the initialize function.

Impact

The Starklane::initialize function will always revert unless the owner specified in the input data is the same as msg.sender.

Tools Used

Manual

Recommendations

To prevent this issue, it is recommended to transfer ownership to msg.sender directly, ensuring that the caller of the initialize function remains the contract owner for subsequent privileged operations.

Updates

Lead Judging Commences

n0kto Lead Judge 9 months ago
Submission Judgement Published
Invalidated
Reason: Non-acceptable severity
Assigned finding tags:

invalid-initialize-owner-has-to-be-msg-sender

No real impact. It even prevents to set an invalid owner. Future versions/upgrades are out-of-scope since this function can/will change to do not modify the owner at every upgrades.

Support

FAQs

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