neon2835 - The _requireOnlyOperatorOrOwnerOf function in the MlumStaking contract has a vulnerability, and users can bypass the judgment

[sherlock-admin4]

neon2835

Medium

The _requireOnlyOperatorOrOwnerOf function in the MlumStaking contract has a vulnerability, and users can bypass the judgment

Summary

The _requireOnlyOperatorOrOwnerOf function in the MlumStaking contract has a vulnerability, and anyone can bypass this function

Vulnerability Detail

The code for the _requireOnlyOperatorOrOwnerOf function is as follows:

function _requireOnlyOperatorOrOwnerOf(uint256 tokenId) internal view {
    // isApprovedOrOwner: caller has no rights on token
    require(ERC721Upgradeable._isAuthorized(msg.sender, msg.sender, tokenId), "FORBIDDEN");
}

Continue to track the ERC721Upgradeable._isAuthorized function:

    function _isAuthorized(address owner, address spender, uint256 tokenId) internal view virtual returns (bool) {
        return
            spender != address(0) &&
            (owner == spender || isApprovedForAll(owner, spender) || _getApproved(tokenId) == spender);
    }

Because in the function _requireOnlyOperatorOrOwnerOf, the first two parameters passed to the _isAuthorized function are both msg.sender, so owner equals spender inside the _isAuthorized function, so the _isAuthorized function always returns true!

Impact

The function _requireOnlyOperatorOrOwnerOf is invalid

Code Snippet

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

Tool used

Manual Review

Recommendation

Modify and optimize

function _requireOnlyOperatorOrOwnerOf(uint256 tokenId) internal view {
    // isApprovedOrOwner: caller has no rights on token
  require(ERC721Upgradeable._isAuthorized(_ownerOf(tokenId), msg.sender, tokenId), "FORBIDDEN");
}

Duplicate of #378