First Flight #12: Kitty Connect

Summary

Anyone can call the ERC721::transferFrom function to transfer cat's tokenId, bypassing the access control in KittyConnect::safeTransferFrom function and breaking the internal logic of user's owned cats.

Vulnerability Details

The KittyConnect contract overrides the safeTransferFrom function to include custom logic that enforces only shop partners can execute transfers and updates the ownership history. Also, the KittyConnect::safeTransferFrom function can only be called by onlyShopPartner. But users can use the ERC721::transferFrom function to transfer tokenId of a cat without any access control.
Using the transferFrom function would bypass the custom logic in KittyConnect::safeTransferFrom function and the onlyShopPartner security check, leading to potential state inconsistencies and unauthorized transfers.

Impact

The following test function test_transferFrom demonstrates that the user who has 2 cat's tokenIds transfers the ownership of one of them to the partnerA (or another user) using the transferFrom function. In that case the list of cats owned by the user will be not updated. The user will own still 2 cats and the partnerA will own 0 cats. You can execute the test using the foundry command: forge test --match-test "test_transferFrom" -vvvvv

If someone uses ERC721::transferFrom function to transfer the tokenId of a cat, the custom logic for cat's ownership in the overridden safeTransferFrom function would be bypassed. Additionally, the security measures enforced by the onlyShopPartner modifier would be circumvented and everyone can transfer the ownership of a cat.

Tools Used

Manual Review, Foundry

Recommendations

Override the ERC721::transferFrom function in the KittyConnect to prevent unauthorized transfer of cat's token: