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

Use of `_mint` instead of `_safe_mint` in Starknet can lead to locking up of NFTs in recipient contracts

hydrapenta

Summary

The withdraw_auto_from_l1 function and the withdrawTokens function in the bridge implementation use the mint function instead of the safe_mint/safeMint function to mint NFTs. This practice bypasses important safety checks that ensure the recipient contract is capable of handling ERC721 tokens, potentially resulting in NFTs being transferred to contracts that cannot process them, leading to permanent loss of those NFTs.

Vulnerability Details

The ERC721 standard provided by OpenZeppelin includes two functions for minting tokens: mint and safe_mint/safeMint. The safe_mint function includes a critical check that ensures the recipient contract can handle ERC721 tokens by verifying the implementation of the onERC721Received function. This is essential to prevent NFTs from being sent to contracts that are not designed to handle them.

if (req.uris.len() != 0) {
let token_uri = req.uris[i];
IERC721BridgeableDispatcher { contract_address: collection_l2 }
.mint_from_bridge_uri(to, token_id, token_uri.clone());
} else {
IERC721BridgeableDispatcher { contract_address: collection_l2 }
.mint_from_bridge(to, token_id);
}

mint``frombridge_uses the _mint instead of __safemint

fn mint_from_bridge(ref self: ContractState, to: ContractAddress, token_id: u256) {
assert(
starknet::get_caller_address() == self.bridge.read(),
'ERC721: only bridge can mint'
); //@note only bridge can
​
self.erc721._mint(to, token_id); //@audit use _safe_mint to prevent trapping of NFT's
}

Same issue exists in the ERC721Bridgeable::mintFromBridgecalled from within the Bridge::withdrawTokensuses mintinstead of safeMint. This approach skips the necessary contract checks, which could result in NFTs being sent to non-compliant contracts, leading to potential permanent loss of the tokens.

function withdrawTokens(
uint256[] calldata request
)
external
payable
returns (address)
{
...
if (!wasEscrowed) {
IERC721Bridgeable(collectionL1).mintFromBridge(req.ownerL1, id);
}
...
}
​
function mintFromBridge(address to, uint256 id)
public
onlyOwner {
​
_mint(to, id);
}

Impact

By using mint instead of safe_mint, NFTs may be permanently lost if they are transferred to a contract that does not support the ERC721Receiver interface, as the tokens may become inaccessible. The absence of contract checks when minting tokens increases the risk of tokens being transferred to unintended or non-compliant recipients

Tools Used

Manual Review.

Recommendations

  • Update the withdraw_auto_from_l1 function to use safe_mint instead of mint to ensure that NFTs are only transferred to addresses capable of handling them

  • Modify the mintFromBridge function in the IERC721Bridgeable implementation to use _safemint instead of _mint:

Updates

Lead Judging Commences

n0kto Lead Judge about 1 year 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.