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

possible reentrancy in `withdrawTokens` function

745fe9f9c2

Summary

see Vulnerability Details

Vulnerability Details

There is a reentrancy vulnerability in the safeTransferFrom function

function withdrawTokens(
uint256[] calldata request
)
external
payable
returns (address)
{
--snip---
for (uint256 i = 0; i < req.tokenIds.length; i++) {
uint256 id = req.tokenIds[i];
​
bool wasEscrowed = _withdrawFromEscrow(ctype, collectionL1, req.ownerL1, id);
​
---snip---
}

Since the withdrawTokens function does not use the noReenter modifier, the attacker can re-enter the withdrawTokens function in the safeTransferFrom function.

cuz of _checkOnERC721Received make a call back to this contract .

function _withdrawFromEscrow(
CollectionType collectionType,
address collection,
address to,
uint256 id
)
internal
returns (bool)
{
---snip---
if (collectionType == CollectionType.ERC721) {
IERC721(collection).safeTransferFrom(from, to, id); // @audit here
} else {
// TODO:
// Check here if the token supply is currently 0.
IERC1155(collection).safeTransferFrom(from, to, id, 1, "");
}
---snip---

in ERC721.sol

function safeTransferFrom(address from, address to, uint256 tokenId) public virtual override {
safeTransferFrom(from, to, tokenId, "");
}
​
/**
* @dev See {IERC721-safeTransferFrom}.
*/
function safeTransferFrom(address from, address to, uint256 tokenId, bytes memory data) public virtual override {
require(_isApprovedOrOwner(_msgSender(), tokenId), "ERC721: caller is not token owner or approved");
_safeTransfer(from, to, tokenId, data);
}
​
function _safeTransfer(address from, address to, uint256 tokenId, bytes memory data) internal virtual {
_transfer(from, to, tokenId);
require(_checkOnERC721Received(from, to, tokenId, data), "ERC721: transfer to non ERC721Receiver implementer");
}
function _checkOnERC721Received(
address from,
address to,
uint256 tokenId,
bytes memory data
) private returns (bool) {
if (to.isContract()) {
try IERC721Receiver(to).onERC721Received(_msgSender(), from, tokenId, data) returns (bytes4 retval) {
return retval == IERC721Receiver.onERC721Received.selector;
} catch (bytes memory reason) {
if (reason.length == 0) {
revert("ERC721: transfer to non ERC721Receiver implementer");
} else {
/// @solidity memory-safe-assembly
assembly {
revert(add(32, reason), mload(reason))
}
}
}
} else {
return true;
}

Impact

possible loss of nft in contract

Tools Used

Manual Review

Recommendations

add nonReentrant to withdrawTokens function

Updates

Lead Judging Commences

n0kto Lead Judge 11 months ago
Submission Judgement Published
Invalidated
Reason: Lack of quality

Appeal created

745fe9f9c2 Submitter
11 months ago
n0kto Lead Judge
11 months ago
n0kto Lead Judge 11 months ago
Submission Judgement Published
Invalidated
Reason: Lack of quality

Support

FAQs

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