First Flight #12: Kitty Connect
First Flight #12: Kitty Connect
Beginner FriendlyFoundryNFTGameFi
100 EXP
View results
Previous Next
Submission Details
Severity: high
Valid

Failure to update owner's token ID array in `mintBridgedNFT` leads to ownership tracking inconsistencies and lost Kitties

irondevx

Summary

When bridging, users can lose their Kitties due to a lacking state update when the bridged Kitty gets minted.

Vulnerability Details

The mintBridgedNFT function is designed to mint a new Kitty token upon receiving a CCIP message from another chain. It assignes the next available index (idx) for the newly minted Kitty, but it does not update the owner's token ID array (s_ownerToCatsTokenId[catOwner]). This oversight can lead to scenarios where two tokens claim the same index position, undermining the integrity of token ownership tracking within the contract.

Impact

All of the bridged Kitties become unavailable upon arrival, because they will either:

  • occupy unavailable indexes (because index = s_ownerToCatsTokenId[catOwner].length isn't reachable yet)

  • overwrite/be overwritten by newly minted Kitties at that exact index, which would cause losses of all Kitties at the same index when bridging to a new chain via bridgeNftToAnotherChain.

Tools Used

Foundry

Proof of Code

Add the following code to the KittyTest.t.sol file:

function test_MintBridgedNFT_UpdatesOwnerTokenArray() public {
vm.prank(kittyConnectOwner);
kittyBridge.allowlistSender(networkConfig.router, true);
​
// ******************** Bridge kitty ********************
bytes memory data = abi.encode(
user,
"Bridged Kitty #1",
"Domestic",
"ipfs://QmbxwGgBGrNdXPm84kqYskmcMT3jrzBN8LzQjixvkz4c62",
block.timestamp,
partnerA
);
​
Client.Any2EVMMessage memory message = Client.Any2EVMMessage({
messageId: bytes32(0),
sender: abi.encode(networkConfig.router),
sourceChainSelector: networkConfig.otherChainSelector,
data: data,
destTokenAmounts: new Client.EVMTokenAmount[](0)
});
​
vm.prank(networkConfig.router);
kittyBridge.ccipReceive(message);
​
// Post-minting, fetch bridged Kitty's token ID and owner's token ID array
uint256 bridgedKittyTokenId = kittyConnect.getTokenCounter() - 1;
uint256[] memory ownerTokenIds = kittyConnect.getCatsTokenIdOwnedBy(
user
);
​
// Check that the bridged Kitty's token ID is present in the owner's array
bool isTokenIdPresent = false;
for (uint256 i = 0; i < ownerTokenIds.length; i++) {
if (ownerTokenIds[i] == bridgedKittyTokenId) {
isTokenIdPresent = true;
break;
}
}
​
assertFalse(
isTokenIdPresent,
"Newly bridged token ID is actually in the owner's token array"
);
assertEq(kittyConnect.getCatInfo(bridgedKittyTokenId).idx, 0);
// *******************************************************
​
// ****** Mint new kitty and override bridged kitty ******
vm.prank(partnerA);
kittyConnect.mintCatToNewOwner(user, "ipfs_hash", "Bob", "Birman", 0);
​
uint256 mintedKittyId = kittyConnect.getTokenCounter() - 1;
ownerTokenIds = kittyConnect.getCatsTokenIdOwnedBy(
user
);
​
// Check that the bridged Kitty's token ID is present in the owner's array
for (uint256 i = 0; i < ownerTokenIds.length; i++) {
if (ownerTokenIds[i] == mintedKittyId) {
isTokenIdPresent = true;
break;
}
}
assertTrue(
isTokenIdPresent,
"Newly minted token ID isn't in the owner's token array"
);
assertEq(kittyConnect.getCatInfo(mintedKittyId).idx, 0);
// *******************************************************
}

Recommendations

Ensure that every token minting operation, including those initiated through the mintBridgedNFT function, correctly updates the s_ownerToCatsTokenId mapping to reflect the new token ID.

function mintBridgedNFT(bytes memory data) external onlyKittyBridge {
// Code above stays the same
​
s_catInfo[tokenId] = CatInfo({
catName: catName,
breed: breed,
image: imageIpfsHash,
dob: dob,
prevOwner: new address[](0),
shopPartner: shopPartner,
idx: s_ownerToCatsTokenId[catOwner].length
});
​
+ s_ownerToCatsTokenId[catOwner].push(tokenId);
​
emit NFTBridged(block.chainid, tokenId);
_safeMint(catOwner, tokenId);
}
Updates

Lead Judging Commences

inallhonesty Lead Judge about 1 year ago
Submission Judgement Published
Validated
Assigned finding tags:

owner's token ID array not updated in `mintBridgedNFT`

Support

FAQs

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