Lack of Listing Check in makePresent Function

Severity

Medium Risk

Relevant GitHub Links

https://github.com/Cyfrin/2024-04-Baba-Marta/blob/main/src/MartenitsaMarketplace.sol#L90-L95

Summary

The makePresent function in the MartenitsaMarketplace.sol contract allows token owners to gift tokens to others without checking if the token is already listed for sale. This oversight can disrupt the logic of the codebase and prevent the purchase of listed tokens.

Vulnerability Details

The makePresent function permits token owners to transfer tokens to others without verifying if the token is already listed for sale in the marketplace. Consequently, an owner can inadvertently gift a token that is listed, which could disrupt the marketplace's functionality and prevent potential buyers from purchasing the listed token.

Proof of the vulnerability is demonstrated in the following test case: to be added to the : MartenitsaMarketplace.t.sol file.

function testMakePresentOfListedItemsAndBuyFails() public {
    // List token for sale
    testListMartenitsaForSale();

    // Gift token
    vm.startPrank(chasy);
    martenitsaToken.approve(address(marketplace), 0);
    marketplace.makePresent(jack, 0);
    vm.stopPrank();

    // Assertions to verify the outcome of the gift transaction
    assert(martenitsaToken.ownerOf(0) == jack);
    assert(martenitsaToken.getCountMartenitsaTokensOwner(bob) == 0);
    assert(martenitsaToken.getCountMartenitsaTokensOwner(jack) == 1);

    // Check if listing still exists
    marketplace.getListing(0);

    // Attempt to buy the token and expect failure
    vm.expectRevert();
    vm.prank(bob);
    marketplace.buyMartenitsa{value: 1 wei}(0);
}

This test case demonstrates that the makePresent function successfully transfers a listed token (tokenId = 0) to another address (jack). Subsequently, an attempt to purchase the token (tokenId = 0) fails, indicating that the token remains listed for sale despite being gifted.

Impact

This vulnerability could lead to disruption in the marketplace's functionality, as listed tokens could be inadvertently gifted, rendering them unavailable for purchase. As a result, buyers would be unable to acquire tokens that were intended for sale, potentially leading to frustration and loss of trust in the marketplace.

Tools Used

manual code review.

Recommendations

To mitigate this vulnerability, it is recommended to implement a check to verify whether the token is listed for sale before allowing it to be gifted. The following fix is proposed:

Check Listing Status: Modify the makePresent function to include a check to verify if the token is listed for sale before allowing it to be transferred as a gift. If the token is listed, the function should revert to prevent unintended disruption of the marketplace's logic.

Example:

function makePresent(address presentReceiver, uint256 tokenId) external {
    require(msg.sender == martenitsaToken.ownerOf(tokenId), "You do not own this token");
    require(!tokenIdToListing[tokenId].forSale, "Token is listed for sale");
    
    // Rest of the function's logic...
}