NFTBridge
60,000 USDC
View results
Submission Details
Severity: medium
Invalid

Bridge can overwrite existing token ownership in `erc721_bridgeable::mint_from_bridge()` and `erc721_bridgeable::mint_from_bridge_uri()`

Summary

Vulnerability Details

The erc721_bridgeable contract implements a bridgeable ERC721 token standard, allowing for cross-chain token transfers. Two key functions, mint_from_bridge() and mint_from_bridge_uri(), are designed to mint new tokens on the Starknet side when tokens are locked on the other side of the bridge. These functions are crucial for maintaining token consistency across chains.

The mint_from_bridge() function is responsible for minting a new token with a specified token_id to a given address. The mint_from_bridge_uri() function extends this functionality by also setting a URI for the newly minted token. Both functions include a check to ensure that only the designated bridge contract can call them, which is a good security measure.

However, a critical oversight exists in both functions: they lack validation to check whether the token_id being minted already exists. This omission creates a significant vulnerability in the token minting process.

The root cause of this issue lies in the direct call to self.erc721._mint() without first verifying the uniqueness of the token_id. This oversight allows the bridge to potentially mint a token with an ID that is already owned by another address, effectively overwriting the ownership of an existing token.

In the worst-case scenario, this issue could be exploited to arbitrarily reassign ownership of any existing token, leading to unauthorized transfers and potential loss of valuable assets for legitimate token holders.

Relevant code:

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'
);
self.erc721._mint(to, token_id); // No validation for existing token ID
}
fn mint_from_bridge_uri(ref self: ContractState, to: ContractAddress, token_id: u256, token_uri: ByteArray) {
IERC721Bridgeable::mint_from_bridge(ref self, to, token_id); // Calls the above function
self.token_uris.write(token_id, token_uri);
}

Impact

  • The lack of token_id validation in mint_from_bridge() and mint_from_bridge_uri() functions can lead to unauthorized overwriting of existing token ownership.

  • The lack of validation for the token_id parameter in the mint_from_bridge and mint_from_bridge_uri functions can lead to unauthorized overwriting of existing tokens.

Proof of Concept

  1. Alice owns a valuable token with token_id 42.

  2. The bridge contract, either due to a malicious action or a bug, calls mint_from_bridge(Bob, 42).

  3. The function does not check if token_id 42 already exists and proceeds to mint the token.

  4. The ownership of token 42 is transferred from Alice to Bob without Alice's consent or any transaction from her side.

  5. Alice has lost her valuable token, and Bob now owns it without having legitimately acquired it.

Tools Used

Manual review

Recommendation

To address this vulnerability, implement a check to ensure that the token_id does not already exist before minting. This can be achieved by using the _exists() function from the ERC721 implementation.

Here's the recommended fix:

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'
);
+ // Check if the token ID already exists
+ assert(
+ !self.erc721._exists(token_id),
+ 'ERC721: token ID already exists'
+ );
self.erc721._mint(to, token_id);
}
fn mint_from_bridge_uri(ref self: ContractState, to: ContractAddress, token_id: u256, token_uri: ByteArray) {
IERC721Bridgeable::mint_from_bridge(ref self, to, token_id);
self.token_uris.write(token_id, token_uri);
}

This change ensures that the bridge cannot mint tokens with IDs that are already in use, preserving the integrity of token ownership and preventing unauthorized transfers.

Updates

Lead Judging Commences

n0kto Lead Judge 12 months ago
Submission Judgement Published
Invalidated
Reason: Incorrect statement

Support

FAQs

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