Summary:
This function makes external calls to ERC721 contracts for transferring or minting tokens after modifying important state, violating the checks-effects-interactions pattern. This implementation creates a risk where malicious contracts could re-enter the function before its initial execution completes, potentially leading to double-spending of tokens or inconsistent state. Given the cross-chain nature of the bridge, this vulnerability could have significant impacts on both Ethereum and Starknet ecosystems, potentially resulting in financial losses and compromising the integrity of the bridging mechanism.
Vulnerability Details:
The withdraw_auto_from_l1
function in the bridge contract performs external calls to other contracts after modifying important state. Specifically, it calls transfer_from
, mint_from_bridge_uri
, or mint_from_bridge
on external contracts without following the checks-effects-interactions pattern. This creates a potential reentrancy vulnerability.
The vulnerable code snippet:
These external calls are made after the function has processed part of the withdrawal request but before it has completed. If the called contracts are malicious or compromised, they could potentially call back into withdraw_auto_from_l1
, leading to unexpected behavior.
Impact:
Double-spending of tokens: An attacker could potentially withdraw the same tokens multiple times.
Inconsistent state: The bridge's state could become inconsistent with the actual token ownership on L2.
Drain of escrowed tokens: In the worst case, an attacker could drain all escrowed tokens from the bridge.
This vulnerability undermines the core functionality of the bridge and could lead to significant financial losses for users and the protocol.
Tools Used:
Manual code review
Static analysis of the Cairo smart contract
Recommendations:
Implement the checks-effects-interactions pattern:
Perform all state changes before making external calls.
Consider using a two-step withdrawal process where the first step marks tokens as "withdrawn" and the second step performs the actual transfer.
Use reentrancy guards:
Implement a reentrancy guard mechanism similar to OpenZeppelin's nonReentrant
modifier in Solidity.
This could involve setting a state variable at the beginning of the function and resetting it at the end, with checks to prevent reentry.
Consider using pull-over-push pattern:
Instead of directly transferring tokens, mark them as available for withdrawal.
Implement a separate function for users to claim their withdrawn tokens.
The contest is live. Earn rewards by submitting a finding.
This is your time to appeal against judgements on your submissions.
Appeals are being carefully reviewed by our judges.