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

User can transfer their own NFT without Shop Partner help by calling `KittyConnect::transferFrom`

farismaulana

Summary

Using KittyConnect::transferFrom inherited from ERC721 contract, user can break the protocol rules stating that only Shop Partner can transfer Cat NFT to another user.
User can transfer their own NFT as they wish.

Vulnerability Details

proof of code

add this code to KittyTest.t.sol:

function test_bypassShopPartnerWhenTransferingNFT() public {
string memory catImageIpfsHash = "ipfs://QmbxwGgBGrNdXPm84kqYskmcMT3jrzBN8LzQjixvkz4c62";
vm.prank(partnerA);
kittyConnect.mintCatToNewOwner(user, catImageIpfsHash, "Meowdy", "Ragdoll", block.timestamp);
uint256 tokenId = kittyConnect.getTokenCounter() - 1;
​
address prevOwner = kittyConnect.ownerOf(tokenId);
​
vm.prank(user);
kittyConnect.transferFrom(user, address(1), tokenId);
​
address newOwner = kittyConnect.ownerOf(tokenId);
​
assert(prevOwner != newOwner);
console.log("owner changed");
}

and then run forge test --mt test_bypassShopPartnerWhenTransferingNFT -vvv the result should PASS:

[PASS] test_bypassShopPartnerWhenTransferingNFT() (gas: 294970)
Logs:
owner changed

Impact

Breaking of protocol rules as this feature is not wanted, user can only transfer their Cat NFT by the help of official Shop Partner

Tools Used

manual review and foundry

Recommendations

overriding the transferFrom function in KittyConnect.sol should prevent the rule breaking bug:

+ function transferFrom(address from, address to, uint256 tokenId) public override {
+ revert("Function disabled, contact nearest Kitty Connect Shop Partner");
+ }
​

and the result of running forge test --mt test_bypassShopPartnerWhenTransferingNFT -vvv should FAIL:

[FAIL. Reason: revert: Function disabled, contact nearest Kitty Connect Shop Partner] test_bypassShopPartnerWhenTransferingNFT() (gas: 282786)
Updates

Lead Judging Commences

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

ERC721 `transferFrom` not overriden

Support

FAQs

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