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

Possible reentrancy in `_withdrawFromEscrow(...)` function

audinarey

Summary

The Escrow::_withdrawFromEscrow(...)function does not implement the checks-effect-interaction pattern and as such it can be reentered by the recipient of the asset

Vulnerability Details

As shown below, the state of the withdraw token idis updated after the safeTransferFromis used to interact with the external contract.

File: Escrow.sol
63: function _withdrawFromEscrow(
64: CollectionType collectionType,
65: address collection,
66: address to,
67: uint256 id
68: )
69: internal
70: returns (bool)
71: {
72: if (!_isEscrowed(collection, id)) {
73: return false;
74: }
75:
76: address from = address(this);
77:
78: if (collectionType == CollectionType.ERC721) {
79: IERC721(collection).safeTransferFrom(from, to, id);
80: } else {
81: // TODO:
82: // Check here if the token supply is currently 0.
83: IERC1155(collection).safeTransferFrom(from, to, id, 1, "");
84: }
85:
86: _escrow[collection][id] = address(0x0);
87:
88: return true;
89: }
​

Impact

possible loss of funds

Tools Used

Manual review

Recommendations

Modify the Escrow::_withdrawFromEscrow(...)function as shown below

File: Escrow.sol
63: function _withdrawFromEscrow(
64: CollectionType collectionType,
65: address collection,
66: address to,
67: uint256 id
68: )
69: internal
70: returns (bool)
71: {
72: if (!_isEscrowed(collection, id)) {
73: return false;
74: }
+75: _escrow[collection][id] = address(0x0);
76: address from = address(this);
77:
78: if (collectionType == CollectionType.ERC721) {
79: IERC721(collection).safeTransferFrom(from, to, id);
80: } else {
81: // TODO:
82: // Check here if the token supply is currently 0.
83: IERC1155(collection).safeTransferFrom(from, to, id, 1, "");
84: }
85:
-86: _escrow[collection][id] = address(0x0);
87:
88: return true;
89: }
Updates

Lead Judging Commences

n0kto Lead Judge 10 months ago
Submission Judgement Published
Invalidated
Reason: Incorrect statement
Assigned finding tags:

finding-withdraw-reentrancy-creates-unbridgeable-tokens

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.

Support

FAQs

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