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

Critical Front-Run Vulnerability in Contract Implementation Change Due to Unprotected Initialization.

0xgenaudits

Summary

The smart contract under review is susceptible to a front-run attack when the implementation is changed.
specifically, during the process of changing the implementation contract, the initialize function becomes
accessible to anyone, creating a critical vulnerability.

Vulnerability Details

When the admin changes the implementation of the contract, the initialize function, which should only be callable by the owner,
becomes exposed. In the event of a front-run attack, a malicious actor can monitor mempool for the implementation change
transaction and call the initialize function within the same block. This would allow the attacker to execute arbitrary code or
reconfigure the contract before the intended administrator can secure the contract by properly initializing it.

POC:

// SPDX-License-Identifier: MIT
pragma solidity ^0.8.0;
import "hardhat/console.sol";
​
contract Test {
​
address _implementation;
mapping(address => bool) public _initializedImpls;
​
function init() public {
_implementation = address(0x1);
initialize();
_implementation = address(0x2);
@>>> // Here attacker can front-run admin.
initialize();
}
​
function initialize() public onlyInit {}
​
modifier onlyInit() {
@>>> address impl = _getImplementation();
@>>> require(!_initializedImpls[impl], "Already init");
_initializedImpls[impl] = true;
_;
}
​
function _getImplementation() internal view returns(address) {
return _implementation;
}
}
@>>> 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);
}
​
modifier onlyInit() {
@>>> address impl = _getImplementation();
@>>> require(!_initializedImpls[impl], "Already init");
_initializedImpls[impl] = true;
_;
}

Impact

The potential impact of this vulnerability is severe:
An attacker can take control of the bridge contract and abuse the protocol.
Users will lost their assets.

Recommendations

To mitigate this vulnerability, it is essential to implement the following measures:

Ensure that the initialize function can only be called by the owner (or a designated administrator) by adding an onlyOwner modifier
to the function. This will prevent unauthorized entities from invoking this function.

- function initialize(bytes calldata data) public onlyInit {
+ function initialize(bytes calldata data) public onlyOwner 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);
}
Updates

Lead Judging Commences

n0kto Lead Judge about 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.

Appeal created

0xgenaudits Submitter
about 1 year ago
0xgenaudits Submitter
about 1 year ago
0xgenaudits Submitter
about 1 year ago
n0kto Lead Judge
about 1 year ago
n0kto Lead Judge about 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.