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

Front-run attack on `initialize` function

danzero

Summary

A malicious actor could front-run the initializefunction in Bridge.sol.

Vulnerability Details

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 initializefunction is public which means anyone can call the function. Attackers could keep track of the contract deployments and be the first to call initialize function which rendered the deployed contract likely malicious, causing the deployer to redeploy the contract which the attacker will keep track of and front-run the initialize call function again and again.

Impact

The attacker gains full control over the contract, including the ability to change critical settings, withdraw funds, or execute privileged functions. Furthermore, a willing attacker could continuously monitor every deployment of this contract and front run the initializefunction causing huge gas cost to be wasted, ultimately a huge financial loss for the deployer.

Tools Used

Manual Review

Recommendations

An alternative would be perform all the initialization on the constructor. But, if the initializefunction must be used no matter what, then consider removing the transfer ownership logic in the initializefunction (this makes the deployer of the contract the current owner because of openzeppelin Ownable modifier), then create a function to use the transferOwnership function from openzeppelin somewhere in Brdige.sol:

function transferContractOwnership(address newOwner) public onlyOwner {
transferOwnership(newOwner);
}

So, after deployment, the deployer of the contract should call this function if any more ownership change is necessary.

Updates

Lead Judging Commences

n0kto Lead Judge over 1 year 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.

Give us feedback!