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

Unrestricted access to contract initialization

Summary

Anyone can call the initialize function before it is initialized.

Vulnerability Details

The `initialize` function is publicly accessible and protected only by the `onlyInit` modifier, which checks if the function has been called before but does not restrict who can call it. This allows any external actor to initialize the contract and set themselves as the owner, along with other critical parameters.

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);
}

https://github.com/Cyfrin/2024-07-ark-project/blob/273b7b94986d3914d5ee737c99a59ec8728b1517/apps/blockchain/ethereum/src/Bridge.sol#L44C4-L67C1

// onlyInit
modifier onlyInit() {
address impl = _getImplementation();
require(!_initializedImpls[impl], "Already init");
_initializedImpls[impl] = true;
_;
}

https://github.com/Cyfrin/2024-07-ark-project/blob/273b7b94986d3914d5ee737c99a59ec8728b1517/apps/blockchain/ethereum/src/UUPSProxied.sol#L19C2-L26C6

An attacker can frontrun the uninitialized contract. The attacker calls the initialize function, providing their own address as the owner. And the attacker can decide to set other parameters to what they are supposed to be.

Impact

Unauthorized actor can become the contract owner.

Tools Used

Manual review

Recommendations

Implement strict access control on the initialize function.

Updates

Lead Judging Commences

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

invalid-bridge-initialize-frontrun

If frontrun at the first deployment, protocol will deploy again, no real impact: informational. Moreover it is already deployed and initialize on mainnet. For the upgrades, `initialize` can/will change for the next update since the owner is already set. A lot of protocol make that change. That’s why I consider it like a future feature and it is out of scope.

Appeal created

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

invalid-bridge-initialize-frontrun

If frontrun at the first deployment, protocol will deploy again, no real impact: informational. Moreover it is already deployed and initialize on mainnet. For the upgrades, `initialize` can/will change for the next update since the owner is already set. A lot of protocol make that change. That’s why I consider it like a future feature and it is out of scope.

Support

FAQs

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