First Flight #12: Kitty Connect

Summary

"Idx" of burned nft won't be switched with "lastItem" popped out of "s_ownerToCatsTokenId" if idx itself is the last item inside of the array after the previous last item was popped out.

Vulnerability Details

Inside "KittyConnect.sol" inside "bridgeNftToAnotherChain()" the "Idx" of burned nft won't be switched with "lastItem" popped out of "s_ownerToCatsTokenId" if idx itself is the last item inside of the array after the previous last item was popped out. To make it more clear if we have s_ownerToCatsTokenIds filled with ids as such (1,2,3,4) and the idx of the nft owner wants to transfer to be equal to 3 (idx=3 with index=2, cause arrays in Solidity are 0 indexed) and we follow the business logic:

to continue the example off-code our idx=3 which is not smaller than 2 (userTokenIds.length is 3-1) meaning we will not go in the if and value=4 (lastItem) will not get to the place of the idx, which leads to the idx staying in the "s_ownerToCatsTokenId" and lastItem being lost.

Impact

High, since the owner of the nft loses one of his nfts and remains with a reference to the id of the one that is burned.

Tools Used

Manual review.

Recommendations

The equation should use equal sign as well in the if statement so: