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

`KittyConnect::safeTransferFrom` does not check whether the proposed new owner is a partner shop

paprikrumplikas

Summary

KittyConnect::safeTransferFrom does not check whether the proposed new owner is a partner shop or not.

Vulnerability Details

Partner shops are NOT supposed to own Kitty NFTs. This is signified by the require statement and custom error in KittyConnect::mintCatToNewOwner:

require(!s_isKittyShop[catOwner], "KittyConnect__CatOwnerCantBeShopPartner");

However, the restriction is not present in KittyConnect::safeTransferFrom, so partner shops can receive NFTs via calling this function.

Proof of Code
function test_NftCanBeTransferredToShop() public {
string memory catImageIpfsHash = "ipfs://QmbxwGgBGrNdXPm84kqYskmcMT3jrzBN8LzQjixvkz4c62";
vm.prank(partnerA); // PartnerA mints a new kitty to User
kittyConnect.mintCatToNewOwner(user, catImageIpfsHash, "Meowdy", "Ragdoll", block.timestamp);
​
uint256 tokenId = 0;
​
vm.prank(user);
kittyConnect.approve(partnerB, tokenId);
​
vm.prank(partnerA);
kittyConnect.safeTransferFrom(user, partnerB, tokenId, "");
​
assert(kittyConnect.ownerOf(0) == partnerB);
}

Impact

Parner shops can receive NFTs while they are not supposed to own any.

Tools Used

Manual review, Foundry.

Recommendations

To prevent shop partners from receiving kitty NFTs, add an additional require statement to KittyConnect::safeTransferFrom as follows:

function safeTransferFrom(address currCatOwner, address newOwner, uint256 tokenId, bytes memory data)
public
override
onlyShopPartner
{
require(_ownerOf(tokenId) == currCatOwner, "KittyConnect__NotKittyOwner");
+ require(!s_isKittyShop[newOwner], "KittyConnect__CatOwnerCantBeShopPartner");
​
require(getApproved(tokenId) == newOwner, "KittyConnect__NewOwnerNotApproved");
​
_updateOwnershipInfo(currCatOwner, newOwner, tokenId); // idx updated here
​
emit CatTransferredToNewOwner(currCatOwner, newOwner, tokenId);
_safeTransfer(currCatOwner, newOwner, tokenId, data);
}
Updates

Lead Judging Commences

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

Shop partner can own cats via safeTransfer

Support

FAQs

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