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

Logic flaw in `bridgeNftToAnotherChain`.

sqrlfirst

Summary

When a user wants to bridge NFT to another chain bridgeNftToAnotherChain function removes tokenId from s_ownerToCatsTokenId. It saves pop the last element in case if idx == length of array or moves the last element to the position of the removed element which is defined by idx filed in catInfo struct. However, in the last case, it doesn't update the idx of the token which leads to removing the wrong element from array if the user wants to then transfer the last element, because idx will be more than the array length.

Impact

High

Recommendations

Update the idx of the last element when removing it.

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;
+ 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.