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

`KittyConnect::bridgeNftToAnotherChain` fails to update `KittyConnect::s_catInfo` storage variable

feder

Summary

KittyConnect::bridgeNftToAnotherChain does not update KittyConnect::s_catInfo storage variable after rearranging elements in KittyConnect::s_ownerToCatsTokenId. As a result, KittyConnect::getCatInfo returns inaccurate data.

Vulnerability Details

While KittyConnect::bridgeNftToAnotherChain takes care of removing the bridged NFT tokenId from KittyConnect::s_ownerToCatsTokenId and replacing it with the last tokenId added to said array (when applicable), it fails to update KittyConnect::s_catInfo (for the moved tokenId) to keep track of the new index position.

As a result, after bridging an NFT, the function KittyConnect::getCatInfo may return inaccurate data.

function bridgeNftToAnotherChain(uint64 destChainSelector, address destChainBridge, uint256 tokenId) external {
address catOwner = _ownerOf(tokenId);
​
require(msg.sender == catOwner);
​
CatInfo memory catInfo = s_catInfo[tokenId];
uint256 idx = catInfo.idx;
bytes memory data = abi.encode(catOwner, catInfo.catName, catInfo.breed, catInfo.image, catInfo.dob, catInfo.shopPartner);
​
_burn(tokenId);
delete s_catInfo[tokenId];
​
uint256[] memory userTokenIds = s_ownerToCatsTokenId[msg.sender];
uint256 lastItem = userTokenIds[userTokenIds.length - 1];
​
s_ownerToCatsTokenId[msg.sender].pop();
​
if (idx < (userTokenIds.length - 1)) {
s_ownerToCatsTokenId[msg.sender][idx] = lastItem;
// @audit s_catInfo[lastItem].idx should be updated to reflect the new index
}
​
emit NFTBridgeRequestSent(block.chainid, destChainSelector, destChainBridge, tokenId);
i_kittyBridge.bridgeNftWithData(destChainSelector, destChainBridge, data);
}

Impact

KittyConnect::getCatInfo may return the wrong idx value for the provided tokenId.

Tools Used

Manual review.

Recommended Mitigation

if (idx < (userTokenIds.length - 1)) {
s_ownerToCatsTokenId[msg.sender][idx] = lastItem;
+ s_catInfo[lastItem].idx = idx;
}
Updates

Lead Judging Commences

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

Consider updating index of an NFT when making a transfer to another user or chain

Support

FAQs

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