sh0velware - safeTransferFrom() function will never work as intended

[sherlock-admin2]

sh0velware

Medium

safeTransferFrom() function will never work as intended

Summary

The safeTransferFrom() function will never work as intended because of the usage of nonReentrantModifier().

Vulnerability Detail

The nonReentrant modifier is used in transferFrom() and safeTransferFrom() functions. Since there is an additional public call to safeTransferFrom(from, to, tokenId, "") in safeTransferFrom(), the nonReentrant modifier will prevent this second call to be performed, leading to an unexpected revert every time that safeTransferFrom() is called.

Impact

The safeTransferFrom() function will revert every time with ReentrancyGuardReentrantCall(), not working as intended.

Code Snippet

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

    /**
     * @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);
    }

Tool used

Manual Review

Recommendation

Remove nonReentrant modifier.

Duplicate of #489