Art Gobblers contest - joestakey's results

Lines of code

https://github.com/code-423n4/2022-09-artgobblers/blob/d2087c5a8a6a4f1b9784520e7fe75afa3a9cbdbe/src/utils/GobblerReserve.sol#L38

Vulnerability details

In GobblerReserve.withdraw, The ArtGobblers.transferFrom() method is used to transfer the gobblers to the recipient to. The problem is that this function does not check whether the recipient can handle ERC721 tokens, which means it could transfer Gobblers to a wallet/contract that cannot handle it, resulting in the loss/freeze of these Gobblers.

Impact

Medium

Tools Used

Manual Analysis

Mitigation

Add one more check in ArtGobblers.transferFrom()

sol
885:         require(from == getGobblerData[id].owner, "WRONG_FROM");
886: 
887:         require(to != address(0), "INVALID_RECIPIENT");
888: 
889:         require(
890:             msg.sender == from || isApprovedForAll[from][msg.sender] || msg.sender == getApproved[id],
891:             "NOT_AUTHORIZED"
892:         );
+ require(
+           to.code.length == 0 ||
+                ERC721TokenReceiver(to).onERC721Received(msg.sender, from, id, "") ==
+                ERC721TokenReceiver.onERC721Received.selector,
+           "UNSAFE_RECIPIENT"
+        );