First Flight #12: Kitty Connect

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

Indices not updated after bridge transfer

Summary

The tokens' indices are not updated when transferred to other chains.

Vulnerability Details

Upon transferring a token to another chain, adjustments are made in the KittyConnect.sol::s_ownerToCatsTokenId variable to remove the transferred item. If the transferred token does not occupy the last position in the array, it is replaced by the last item. However, the code fails to update the new index according to the movement of the item in the list. Subsequently, if this item is later transferred and there are elements to its right, the last element will be removed instead of the corresponding token.

Impact

Using the getter function KittyConnect.sol::getCatsTokenIdOwnedBy() to query the tokens owned by the caller will yield incorrect token IDs due to the misalignment of indices caused by the flawed update mechanism.

Tools Used

Manual review.

Proof of Code

Add the following to the current test suite:

Code
function test_idxNotUpdated() public {
address someUser = makeAddr("someUser");
string
memory catImageIpfsHash = "ipfs://QmbxwGgBGrNdXPm84kqYskmcMT3jrzBN8LzQjixvkz4c62";
vm.prank(partnerA);
kittyConnect.mintCatToNewOwner(
someUser,
catImageIpfsHash,
"Hehe",
"Hehe",
block.timestamp
);
vm.prank(partnerA);
kittyConnect.mintCatToNewOwner(
someUser,
catImageIpfsHash,
"Hehe",
"Hehe",
block.timestamp
);
uint64 otherChainSelector = 14767482510784806043;
address destChainBridge = makeAddr("destChainBridge");
uint256 idxBefore = kittyConnect.getCatInfo(1).idx;
vm.prank(someUser);
kittyConnect.bridgeNftToAnotherChain(
otherChainSelector,
destChainBridge,
0
);
vm.stopPrank();
uint256 idxAfter = kittyConnect.getCatInfo(1).idx;
assertEq(idxBefore, idxAfter);
}

Recommendations

Update the idx accordingly:

if (idx < (userTokenIds.length - 1)) {
+ s_catInfo[lastItem].idx = idx;
s_ownerToCatsTokenId[msg.sender][idx] = lastItem;
}
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.