minhquanym - safeTransferFrom() always revert because of nonReentrant modifier

[sherlock-admin3]

minhquanym

Medium

safeTransferFrom() always revert because of nonReentrant modifier

Summary

MlumStaking override the safeTransferFrom() function but adding a nonReentrant modifier, which will make this function always revert.

Vulnerability Detail

In MlumStaking contract, function safeTransferFrom and transferFrom are overriden to add a nonReentrant modifier to them.

However, because of adding a nonReentrant modifier, the function safeTransferFrom will produce all calls to this function to revert, since internally safeTransferFrom function calls to transferFrom and this functions also implements a nonReentrant modifier.

Impact

safeTransferFrom() always revert.

Code Snippet

https://github.com/sherlock-audit/2024-06-magicsea/blob/main/magicsea-staking/src/MlumStaking.sol#L325-L342
In MlumStaking.sol

/**
 * @dev Add nonReentrant to ERC721.transferFrom
 */
function transferFrom(address from, address to, uint256 tokenId)
    public
    override(ERC721Upgradeable, IERC721)
    nonReentrant
{
    ERC721Upgradeable.transferFrom(from, to, tokenId);
}

/**
 * @dev Add nonReentrant to ERC721.safeTransferFrom
 */
function safeTransferFrom(address from, address to, uint256 tokenId, bytes memory _data)
    public
    override(ERC721Upgradeable, IERC721)
    nonReentrant
{
    ERC721Upgradeable.safeTransferFrom(from, to, tokenId, _data);
}

In OZ lib ERC721Upgradeable.sol

/**
 * @dev See {IERC721-safeTransferFrom}.
 */
function safeTransferFrom(address from, address to, uint256 tokenId, bytes memory data) public virtual {
    transferFrom(from, to, tokenId);
    _checkOnERC721Received(from, to, tokenId, data);
}

Tool used

Manual Review

Recommendation

Consider removing these overriden functions.

Duplicate of #489