slowfi - Improper Access Control on _requireOnlyOperatorOrOwnerOf function

[sherlock-admin4]

slowfi

High

Improper Access Control on _requireOnlyOperatorOrOwnerOf function

Summary

The permission function _requireOnlyOperatorOrOwnerOf on the MlumStaking contract returns always true failing to verify if the msg.sender is authorized for performing operations with the NFT.

Vulnerability Detail

The contract uses the function _isAuthorized incorrectly. The function should use as first parameter the owner, then the spender in order to verify. However the MlumStaking contract sends the msg.sender two times, making the function always return true.

Impact

Improper access control on NFT usage on the MlumStaking contract.

Code Snippet

MlumStaking.sol#L140-L143

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

ERC721Upgradeable contract _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);
  }

Tool used

Manual Review

Recommendation

Send the adequate parameters to the function to avoid malicious access controls.

Duplicate of #378