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

`KittyConnect::mintBridgedNFT()` incorrectly tracks `KittyConnect::s_ownerToCatsTokenId` for bridged NFTs

glitchical

Summary

The KittyConnect::mintBridgedNFT() function fails to update KittyConnect::s_ownerToCatsTokenId mapping leading to an incorrect tracking of owned NFT tokenIds for a user.

Vulnerability Details

The KittyConnect::mintBridgedNFT() function is called by the KittyBridge contract and is used to mint a NFT for when a user decides to bridge their NFT. First the function decodes the data, gets and updates the kittyTokenCounter, sets the s_catInfo, emits an event and finally mints the NFT as you can see here:

function mintBridgedNFT(bytes memory data) external onlyKittyBridge {
(
address catOwner,
string memory catName,
string memory breed,
string memory imageIpfsHash,
uint256 dob,
address shopPartner
) = abi.decode(data, (address, string, string, string, uint256, address));
​
uint256 tokenId = kittyTokenCounter;
kittyTokenCounter++;
​
s_catInfo[tokenId] = CatInfo({
catName: catName,
breed: breed,
image: imageIpfsHash,
dob: dob,
prevOwner: new address[](0),
shopPartner: shopPartner,
idx: s_ownerToCatsTokenId[catOwner].length
});
​
emit NFTBridged(block.chainid, tokenId);
_safeMint(catOwner, tokenId);
}

The issue currently lies around setting the CatInfo idx. The idx is set to be s_ownerToCatsTokenId[catOwner].length which isn't an issue of itself, but s_ownerToCatsTokenId is never updated here. Should a user bridge multiple NFTs over, the idx of those NFTs will be the same. Should a shop mint a new NFT via KittyConnect::mintCatToNewOwner with an existing bridged NFT, it will also share the same idx as the bridged NFT.

It's worth noting, since a bridged NFT isn't updated, the mapping s_ownerToCatsTokenId[catOwner] won't track bridged NFTs for a user.

Impact

  • Allows minted and bridged NFTs to share the same idx.

  • Results in KittyConnect::getCatInfo() to return incorrect data.

  • Results in KittyConnect::getCatsTokenIdOwnedBy() to not return the correct set of tokenIds owned by a address.

POC

function test_incorrectTrackingForBridgedNFT() public {
address owner = makeAddr("owner");
uint256 tokenId = kittyConnect.getTokenCounter(); // Should be 0
bytes memory data = abi.encode(
owner,
"meowdy",
"ragdoll",
"ipfs://QmbxwGgBGrNdXPm84kqYskmcMT3jrzBN8LzQjixvkz4c62",
block.timestamp,
partnerA
);
​
// Pretend to be the bridge and mint for the owner
vm.prank(address(kittyBridge));
kittyConnect.mintBridgedNFT(data);
​
// Let's get info our new bridged NFT
KittyConnect.CatInfo memory catInfo = kittyConnect.getCatInfo(tokenId);
uint256[] memory userCatTokenId = kittyConnect.getCatsTokenIdOwnedBy(
owner
);
​
assert(userCatTokenId.length == 0); // Should be equal to 1, but KittyConnect hasn't pushed the tokenId...
assert(catInfo.idx == 0); // The idx will be 0, which is technically okay since it's the first one...
​
// However, now let's mint a NFT...
vm.prank(partnerA);
kittyConnect.mintCatToNewOwner(
owner,
"ipfs://QmbxwGgBGrNdXPm84kqYskmcMT3jrzBN8LzQjixvkz4c62",
"Hehe",
"Hehe",
block.timestamp
);
​
// Let's get info on our new minted NFT
KittyConnect.CatInfo memory newCatInfo = kittyConnect.getCatInfo(
tokenId + 1
);
uint256[] memory newUserCatTokenId = kittyConnect.getCatsTokenIdOwnedBy(
owner
);
​
assert(newUserCatTokenId.length == 1); // KittyConnect has now pushed this tokenId (should really be 2)...
assert(newCatInfo.idx == 0); // BUT the idx is still 0!
}

Tools Used

VS Code, Foundry

Recommendations

Update s_ownerToCatsTokenId and push the tokenId:

function mintBridgedNFT(bytes memory data) external onlyKittyBridge {
(
address catOwner,
string memory catName,
string memory breed,
string memory imageIpfsHash,
uint256 dob,
address shopPartner
) = abi.decode(data, (address, string, string, string, uint256, address));
​
uint256 tokenId = kittyTokenCounter;
kittyTokenCounter++;
​
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 over 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.