Beginner FriendlyFoundryGameFi
100 EXP
View results
Submission Details
Severity: medium
Valid

transferFrom and safeTransferFrom do not call the updateCountMartenitsaTokensOwner to update user token count

Summary

The MartenitsaMarketplace contract allows users to send tokens to other users using the makePresent function. However, users can also transfer tokens using the MartenitsaToken::transferFrom or MartenitsaToken::safeTransferFrom functions inherited from the ERC721 standard. The issue arises when users utilize transferFrom or safeTransferFrom, as the MartenitsaToken::updateCountMartenitsaTokensOwner mapping is not updated correctly, leading to inconsistencies in token ownership tracking.

Vulnerability Details

The vulnerability stems from the fact that the makePresent function correctly updates the updateCountMartenitsaTokensOwner mapping when a token is sent to another user. However, when users choose to transfer tokens using the transferFrom or safeTransferFrom functions, the mapping is not updated accordingly. This results in a discrepancy between the actual token ownership and the ownership recorded in the updateCountMartenitsaTokensOwner mapping.

Impact

The impact of this vulnerability is twofold:

  1. Inconsistent Token Ownership Tracking: The updateCountMartenitsaTokensOwner mapping, which is intended to keep track of the number of tokens owned by each user, becomes unreliable when tokens are transferred using transferFrom or safeTransferFrom. This can lead to incorrect token counts and confusion regarding the true ownership of tokens.

  2. Potential Exploitation: Malicious users may exploit this vulnerability by intentionally using transferFrom or safeTransferFrom to transfer tokens without updating the updateCountMartenitsaTokensOwner mapping. This can be used to manipulate token ownership records and potentially gain unauthorized access to certain functionalities or benefits associated with token ownership.

POC

In the test below, bob has one token before the transfer and has one token after the transfer

function testTransferToken() public hasMartenitsa {
vm.startPrank(bob);
martenitsaToken.approve(address(marketplace), 0);
assert(martenitsaToken.getCountMartenitsaTokensOwner(bob) == 1);
martenitsaToken.transferFrom(bob, jack, 0);
vm.stopPrank();
assert(martenitsaToken.getCountMartenitsaTokensOwner(bob) == 1);

Tools Used

Manual Review

Recommendations

To address this vulnerability, it is recommended to take the following actions:

  1. Update the transferFrom and safeTransferFrom functions: Modify the transferFrom and safeTransferFrom functions to include the necessary logic to update the updateCountMartenitsaTokensOwner mapping whenever a token is transferred. This ensures that the mapping remains consistent with the actual token ownership.

  2. Implement Access Control: Consider implementing access control mechanisms to restrict the usage of transferFrom and safeTransferFrom functions to authorized entities or specific conditions. This can help prevent unauthorized transfers and maintain the integrity of the token ownership records.

Updates

Lead Judging Commences

bube 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.