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

Unsafe usage of _mint()

asda

Summary

The protocol mints the token as an representation of the nft on the blockchain which uses _mint()function which is not recommended and does not have proper precautions to make sure the nftisn't lost.

Vulnerability Details

The vulnerable line of code can be find below, the function checks if the nft has been escrowed or not, meaning if the asset is being hold by escrow contract if not it calls mintFromBridge().

// Bridge.sol
if (!wasEscrowed) {
// TODO: perhaps, implement the same interface for ERC721 and ERC1155
// As we only want to deal with ERC1155 token with value = 1.
// Also, check what to do with URIs. If the URI storage is supported
// or not for ERC721. If supported, we may need to mint with an URI.
IERC721Bridgeable(collectionL1).mintFromBridge(req.ownerL1, id);
}
// IERC721Bridgeable.sol
function mintFromBridge(address to, uint256 id)
public
onlyOwner {
​
_mint(to, id);
}
function _mint(address to, uint256 tokenId) internal virtual {
require(to != address(0), "ERC721: mint to the zero address");
require(!_exists(tokenId), "ERC721: token already minted");
​
_beforeTokenTransfer(address(0), to, tokenId, 1);
​
// Check that tokenId was not minted by `_beforeTokenTransfer` hook
require(!_exists(tokenId), "ERC721: token already minted");
​
unchecked {
// Will not overflow unless all 2**256 token ids are minted to the same owner.
// Given that tokens are minted one by one, it is impossible in practice that
// this ever happens. Might change if we allow batch minting.
// The ERC fails to describe this case.
_balances[to] += 1;
}
​
_owners[tokenId] = to;
​
emit Transfer(address(0), to, tokenId);
​
_afterTokenTransfer(address(0), to, tokenId, 1);
}

As you can notice there are no checks to see if the receiver of the NFT is a valid receiver which can lead the NFT to be stuck.


Impact

NFT can be stuck and a loss to the owner

Tools Used

Manual

Recommendations

Use _safeMint() instead

Updates

Lead Judging Commences

n0kto Lead Judge 12 months ago
Submission Judgement Published
Invalidated
Reason: Non-acceptable severity
Assigned finding tags:

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.

Support

FAQs

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