Use safe_transfer_from instead of transfer_from in withdraw_auto_from_l1
Summary
It is recommended to use safe_transfer_from instead of transfer_from when transferring ERC721s out of the bridge contract.
Vulnerability Details
In withdraw_auto_from_l1 the escrowed tokens are transfered using transfer_from. But it is better to use safe_transfer_from.
The recipient could have logic in the on_erc721_received function, which is only triggered in the safe_transfer_from function and not in transfer_from. It helps ensure that the recipient is indeed capable of handling ERC721s.
Additionally if req.owner_l2 address which is set while bridging from L1, is set to an contract which doesn't support ERC721, then NFTs might lost forever.
Using safe_transfer_from will ensure the handler reverts in such cases. Later the message can be cancelled to get the locked tokens on L1 side
Impact
NFTs might be lost forever and doesn't ensure recipient is capable of handling ERC721s.
Tools Used
Manual review
Recommendations
Use safe_transfer_from instead of transfer_from
Lead Judging Commences
Informational / Gas
Please, do not suppose impacts, think about the real impact of the bug and check the CodeHawks documentation to confirm: https://docs.codehawks.com/hawks-auditors/how-to-determine-a-finding-validity A PoC always helps to understand the real impact possible.
Appeal created
Informational / Gas
Please, do not suppose impacts, think about the real impact of the bug and check the CodeHawks documentation to confirm: https://docs.codehawks.com/hawks-auditors/how-to-determine-a-finding-validity A PoC always helps to understand the real impact possible.
Prize pool breakdown
Live
The contest is live. Earn rewards by submitting a finding.
Appeals
This is your time to appeal against judgements on your submissions.
Appeals Review
Appeals are being carefully reviewed by our judges.