First Flight #13: Baba Marta
First Flight #13
Beginner FriendlyFoundryGameFi
100 EXP
View results
Previous Next
Submission Details
Severity: low
Invalid

Missing `onERC721Received` hooks in buyer smart contract wallets, causes revert in `MartenitsaMarketplace::buyMartenitsa` function.

bluedragon

Description

The MartenitsaMarketplace::buyMartenitsa function uses the safeTransferFrom function to transfer the martenitsaToken from the seller to the buyer. If the buyer contract does not implement the onERC721Received function, the safeTransferFrom function will revert, preventing the buyer from purchasing the martenitsaToken.

function buyMartenitsa(uint256 tokenId) external payable {
.
.
.
.
// Transfer the token to the buyer
@> martenitsaToken.safeTransferFrom(seller, buyer, tokenId);
}

Impact

If the buyer contract does not implement the onERC721Received function, the safeTransferFrom function will revert, preventing the buyer from purchasing the martenitsaToken. This can lead to a poor user experience and may result in failed transactions.

Proof of Concept:

  1. A buyer contract is created without implementing the onERC721Received function.

  2. The buyer contract attempts to purchase a martenitsaToken from the marketplace.

  3. The safeTransferFrom function reverts due to the lack of onERC721Received implementation in the buyer contract.

  4. The transaction fails, and the buyer is unable to purchase the martenitsaToken.

Proof Of Code
function test_BuyerWithoutHooksReverts() public listMartenitsa {
// approve marketplace to spend martenitsa token
vm.prank(chasy);
martenitsaToken.approve(address(marketplace), 0);
​
// initialize the buyer contract
BuyerWithoutHooks buyerWithoutHooks = new BuyerWithoutHooks(address(marketplace));
vm.deal(address(buyerWithoutHooks), 1 wei);
​
// buy martenitsa
vm.startPrank(address(buyerWithoutHooks));
console.log("The address of seller: ", chasy);
console.log("The owner of the token before buying martenitsa: ", martenitsaToken.ownerOf(0));
console.log("The balance of the buyer before buying martenitsa: ", address(buyerWithoutHooks).balance);
​
// As the buyer doesnt have onERC721Received function, the buyMartenitsa function reverts
vm.expectRevert();
​
buyerWithoutHooks.buyMartenitsa(0);
console.log("The balance of the buyer after buying martenitsa: ", address(buyerWithoutHooks).balance);
console.log("The owner of the token after buying martenitsa: ", martenitsaToken.ownerOf(0));
vm.stopPrank();
}

Here is the contract as well

contract BuyerWithoutHooks {
MartenitsaMarketplace public marketplace;
​
constructor(address _marketplace) {
marketplace = MartenitsaMarketplace(_marketplace);
}
​
function buyMartenitsa(uint256 tokenId) public {
marketplace.buyMartenitsa{value: 1 wei}(tokenId);
}
​
receive() external payable {}
}

Here is the foundry output

Logs:
The address of seller: 0x4f19A0AcB98b7aA30b0138D26e5E5F9634F32F5A
The owner of the token before buying martenitsa: 0x4f19A0AcB98b7aA30b0138D26e5E5F9634F32F5A
The balance of the buyer before buying martenitsa: 1
The balance of the buyer after buying martenitsa: 1
The owner of the token after buying martenitsa: 0x4f19A0AcB98b7aA30b0138D26e5E5F9634F32F5A

Recommended Mitigation

  1. Ensure that the buyer contract implements the onERC721Received function as required by the ERC-721 standard. This function is called by the ERC-721 token contract after a successful transfer of a token to the buyer contract.

  2. If the buyer contract does not implement the onERC721Received function, consider using the transferFrom function instead of safeTransferFrom to transfer the token. The transferFrom function does not require the buyer contract to implement the onERC721Received function and will not revert if the function is missing.

  3. Provide clear documentation and guidance to users on the requirements for buying tokens from the marketplace, including the need to implement the onERC721Received function in the buyer contract.

  4. Consider implementing checks in the MartenitsaMarketplace contract to detect whether the buyer contract implements the onERC721Received function and provide appropriate feedback to the user if the function is missing.

contract BuyerWithoutHooks {
MartenitsaMarketplace public marketplace;
​
constructor(address _marketplace) {
marketplace = MartenitsaMarketplace(_marketplace);
}
​
function buyMartenitsa(uint256 tokenId) public {
marketplace.buyMartenitsa{value: 1 wei}(tokenId);
}
​
+ function onERC721Received(address, address, uint256, bytes memory) public virtual returns (bytes4) {
+ return this.onERC721Received.selector;
}
​
receive() external payable {}
}
Updates

Lead Judging Commences

bube Lead Judge over 1 year ago
Submission Judgement Published
Invalidated
Reason: Non-acceptable severity

Support

FAQs

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