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

Use of `CREATE` opcode is suspicious of reorg attack

Github

https://github.com/Cyfrin/2024-07-ark-project/blob/a1e10ca451f9e24d33b4418009eece666f14f15e/apps/blockchain/ethereum/src/token/Deployer.sol#L30

Summary

The deployERC721Bridgeable function deploys a UUPS proxied ERC721 contract using the CREATE opcode, which is vulnerable to reorg attacks. The standard CREATE method relies on the nonce of the transaction sender (the Deployer library contract in this case), making it risky in the event of a reorg.

Vulnerability Details

The ArkProject NFT Bridge allows users to transfer Everai NFTs between Ethereum (L1) & Starknet (L2). It relies on the Starknet messaging protocol for this bridging process. The key components include the bridge admin who manages the bridge settings and users who perform the bridging of NFTs.

When withdrawTokens is called, it allows users to withdraw NFTs from L2 to L1. This function calls _deployERC721Bridgeable, which deploys a new ERC721Bridgeable contract on L1 if the corresponding contract doesn't exist. _deployERC721Bridgeable then calls deployERC721Bridgeable, which uses the CREATE opcode to deploy the proxy contracts. The CREATE opcode is problematic because it is vulnerable to reorgs.

ArkProject will be deployed only on Ethereum and Starknet, and reorgs can occur on these L1 and L2 chains respectively. E.g, here is a previous reorg that happened on Ethereum: Ethereum reorg example - 2 years ago.

The vulnerability here is that users rely on address derivation in advance or when trying to deploy the same address on different chains. Any funds requested for the withdrawal can be stolen due to reorg manipulation.

Impact

A malicious actor can exploit reorg events to deploy a malicious contract at the address intended for legitimate use, enabling them to steal NFTs during the withdrawal process from Starknet (L2) to Ethereum (L1).

Proof of Concept

  1. Alice, a legitimate user, wants to withdraw her Everai NFT from Starknet (L2) to Ethereum (L1).

  2. The bridge admin has enabled the bridge and the whitelist for the Everai NFT collection.

  3. Bob, a malicious actor, sets up a bot that monitors the Ethereum blockchain for reorg events.

  4. Bob knows that during a reorg, the nonce of the deploying contract can change, affecting the address generated by the CREATE opcode.

  5. Alice initiates the withdrawTokens function to withdraw her NFT.

  6. The contract checks the message, and since the corresponding L1 contract doesn't exist, it prepares to deploy a new ERC721Bridgeable contract using CREATE.

  7. Just as Alice's transaction is about to be mined, a reorg occurs.

  8. Bob's bot detects the reorg and quickly deploys his own malicious ERC721Bridgeable contract to the address that the ArkProject NFT Bridge contract would use post-reorg.

  9. Bob’s contract mimics the interface of the legitimate ERC721Bridgeable contract but includes a malicious backdoor.

  10. The reorg completes, and Alice's transaction is mined. Due to the reorg, the nonce has changed, and the contract address generated by Alice's transaction now points to Bob's malicious contract.

  11. Alice’s transaction finalizes, unknowingly interacting with Bob’s contract.

  12. Alice's NFT is transferred to Bob’s malicious contract.

  13. Bob’s contract, using the backdoor, transfers the NFT to Bob’s address.

  14. Bob successfully steals Alice’s NFT.

  15. Alice is unaware that her NFT has been diverted to a malicious contract due to the reorg and nonce manipulation.

Recommendation

To prevent this attack, the bridge should use CREATE2 with a salt to ensure that the contract address is deterministic and not dependent on the nonce, which can change during reorgs.

This is how it should be implemented with CREATE2

Modify the deployERC721Bridgeable function to use CREATE2:

function deployERC721Bridgeable(
string memory name,
string memory symbol,
bytes32 salt
)
public
returns (address)
{
address impl = address(new ERC721Bridgeable());
bytes memory dataInit = abi.encodeWithSelector(
ERC721Bridgeable.initialize.selector,
abi.encode(name, symbol)
);
address proxy;
bytes memory bytecode = abi.encodePacked(
type(ERC1967Proxy).creationCode,
abi.encode(impl, dataInit)
);
assembly {
proxy := create2(0, add(bytecode, 0x20), mload(bytecode), salt)
if iszero(proxy) {
revert(0, 0)
}
}
return proxy;
}

Update _deployERC721Bridgeable to pass the required salt:

function _deployERC721Bridgeable(
string memory name,
string memory symbol,
snaddress collectionL2,
uint256 reqHash
)
internal
returns (address)
{
bytes32 salt = keccak256(abi.encodePacked(msg.sender, reqHash));
address proxy = Deployer.deployERC721Bridgeable(name, symbol, salt);
_l1ToL2Addresses[proxy] = collectionL2;
_l2ToL1Addresses[collectionL2] = proxy;
emit CollectionDeployedFromL2(
reqHash,
block.timestamp,
proxy,
snaddress.unwrap(collectionL2)
);
return proxy;
}

This approach will enhance the security of the ArkProject NFT Bridge by mitigating the risk of reorg attacks.

Updates

Lead Judging Commences

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

invalid-reorg-attack-create

`_l1ToL2Addresses` and `_l2ToL1Addresses` can only be set by the bridge owner or at the deployment in `withdrawTokens`. Even if there is a reorg, the contract will be deployed on a new address which won’t change anything to bridge tokens. No impact.

Support

FAQs

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