When withdrawing tokens, these tokens are either escrowed by the bridge or not. If the bridge holds the token, we transfer it from the bridge to the ownerL1
, else this means that the NFT collection is a custom NFT collection deployed by the bridge and we call ERC721Collection::mintFromBridge
to mint it.
If the NFT is escrowed, we transfer it to the owner and burn it (setting escrow to address_zero
).
As we can see we are transferring the NFT using safeTransferFrom
. Then, burning it and this opens up a Reentrancy!
since safeTransferFrom
calls onERC721Received()
, the user can do whatever he wants before the NFT gets burned. now what if the user deposited the NFT to receive it on L2
again via calling L2Bridger::depositTokens()
?
When depositing we are escrowing the tokenId(s) we are depositing, where the sender sends the token(s) to the bridge then it is escrowed.
Now since we called this function in onERC721Received()
when we return from it, we will return back to the point of withdrawing escrowed tokens, where we will set escrow to address(0)
. Now this will result in resetting the escrow mapping for that token into address(0)
although it is owned by the L1Bridge
.
We are withdrawing escrowed tokens when either Bridging from L2
to L1
and tokens are escrowed, or we are canceling Message Requests from L1
to L2
that the Starknet sequencer did not process, we will illustrate the two attacks.
Canceling Message:
UserA deposited an NFT token and provided less msg.value
to the StarknetMessaging protocol, using a contract implementing onERC721Received()
that calls L1Bridge::depositTokens()
again.
The sequencer didn't process the message because of low gas paid.
UserA messaged the protocol to cancel his message.
The protocol started canceling requests (there is nothing weird till this point).
UserA called cancelRequest after the period has been passed.
Tokens are transferred to the UserA msg.sender
, which is the contract implementing that onERC721Received()
.
The token gets redeposited into L1Bridge
to be Bridged into the user on L2
when calling onERC721Received()
, but he provided correct data and enough gas this time.
onERC721Received()
finished.
L1Bridge
reset that token escrow into address(0)
.
UserA received that NFT on L2
.
UserA sold this NFT to UserB on L2
.
Time passes, and UserB tries to Bridge his NFT to L1
.
When withdrawing, the L1Bridge
found that the NFT was not escrowed so he tried mint it.
The token holder is actually the bridge. so minting will revert because the token already existed.
UserB lost his NFT forever, and any other tokens bridged with that token will get stuck, as the message will always revert when trying to execute it on L1Bridge
and withdraw tokens.
Bridging from L2
to L1
:
It is the same scenario except for doing the attack when canceling the message, the User will need to Bridge his token to L2
then bridge it back to L1
and do the attack when withdrawing it from L1
bridge (the token is escrowed by the bridge when he bridged it first time from L1
to L2
).
This attack costs more for the attacker, but the user don't have to message Protocol admins to Start canceling his message.
We wrote a script that simulates the first attack (Canceling Message
).
Add this in the last of the apps/blockchain/ethereum/test/Bridge.t.sol
file.
Since escrow
variable is private, you will need to make it a public
visibility in order for the test to work, add the public visibility to it.
On the cmd write the following command.
Output:
The POC checks that the Bridge will end up being the owner of the tokens, and they are not escrowed, which means anyone who tries to withdraw it back when bridging from L2
to L1
will end up trying minting it which will revert.
An innocent user can lose his NFTs on L2
when Bridging them back to L1
.
Manual Review + Foundry
Implement the CEI pattern by burning The token before transferring it.
Impact: - NFT already bridged won’t be bridgeable anymore without being stuck. Likelyhood: Low. - Attackers will corrupt their own tokens, deploying a risky contract interacting with an upgradable proxy. They have to buy and sell them without real benefits, except being mean. Some really specific and rare scenario can also trigger that bug.
Impact: - NFT already bridged won’t be bridgeable anymore without being stuck. Likelyhood: Low. - Attackers will corrupt their own tokens, deploying a risky contract interacting with an upgradable proxy. They have to buy and sell them without real benefits, except being mean. Some really specific and rare scenario can also trigger that bug.
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.