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

mint_from_bridge can lock NFTs in owner

SBSecurity

Summary

Usage of mint instead of safeMint in both Bridge and bridge.cairo can lock the tokens in the recipient if he doesn’t expect to receive ERC721 tokens.

Vulnerability Details

Important note: nothing related to safeMint instead of mint was observed in the known issues.

We can see that both bridge contracts will mint the NFTs to the recipient in case it is not escrowed, i.e. this is the first time this collection is being bridged to the chain.

The _safeMint() includes a necessary safety check that validates a recipient contract’s ability to receive and handle ERC-721 tokens. Without this safeguard, tokens can inadvertently be sent to an incompatible contract, causing them, and any assets they hold, to become irretrievable. If the safe operation is used transactions will fail in the bridge operation and that will give the ability originator chain senders to cancel the transaction and retrieve their assets, while now this is not possible since transaction will be completed successfully and prover will update the state so no cross chain transaction cancellation is possible.

Bridge.sol

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

bridge.cairo

#[l1_handler]
fn withdraw_auto_from_l1(
ref self: ContractState,
from_address: felt252,
req: Request
) {
...MORE CODE
} else {
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);
}
}
​
i += 1;
};
}

Impact

Irrevocable NFTs due to usage of mint instead of safeMint which will cause loss of all the NFTs in case receiver chain recipient is not able to handle ERC721 tokens.

Tools Used

Manual Review

Recommendations

Replace mint with safeMint, no reentrancy can be caused in any part of the codebase.

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.

Appeal created

slavcheww Submitter
about 1 year ago
n0kto Lead Judge
12 months ago
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.