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

Lack of Input Validation on initialize function

rea

Summary

If a malicious actor can call initialize with incorrect addresses, the contract could be directed to send messages to malicious addresses, resulting in token loss or incorrect operations.

Vulnerability Details

The initialize function does not validate critical parameters like starknetCoreAddress, starklaneL2Address, and starklaneL2Selector. This can lead to the contract being initialized with malicious or incorrect addresses, compromising its integrity.

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

Impact

Losses could include the entire amount of tokens handled by the bridge if the bridge is pointed to a malicious contract.

PoC

Step 1: A malicious actor calls the initialize function with arbitrary or malicious addresses.

Step 2: The contract stores these addresses without validation.

Step 3: All future interactions with the StarkNet bridge could be compromised, leading to potential loss of user funds or the contract being redirected to a malicious entity.

CODE SNIPPET

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

Tools Used

Manual review

Recommendations

Add validation checks to ensure that the provided addresses are legitimate and belong to trusted contracts.

function initialize(
bytes calldata data
)
public
onlyInit
{
(
address owner,
IstarknetMessaging
starknetCoreAdress,
uint256 starklaneL2Address,
uint256 starklaneL2Selector
) = abi.decode(
data,
(address,IStarknetMessaging,
uint256,uint256)
);
​
require(owner! = address(0),"invalid owner address");
require(address(starknetCoreAdress)! = address(0),"invalid Starknet Core address");
require(starklaneL2Address! =0,"invalid Starklane L2 address");
require(starklaneL2Selector! =0,"invalid Starklane L2 selector");
​
_enabled = false;
_starknerCoreAddress = starknetCoreAddress;
​
_transferOwnership(owner);
​
setStarklaneL2Address(starklaneL2Address);
​
setStarklaneL2Selector(starklaneL2Selector);
}
Updates

Lead Judging Commences

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