MlumStaking.sol :: Anyone can add to the stakedPosition resulting the lockTime of the actual owner to be extended to undefined time

Summary

Anyone can add to the stakedPosition resulting the lockTime to be extended indefinitely.

Vulnerability Detail

We are calling _requireOnlyOperatorOrOwnerOf to check themsg.sender is either the owner or the operator.

function addToPosition(uint256 tokenId, uint256 amountToAdd) external override nonReentrant {
        _requireOnlyOperatorOrOwnerOf(tokenId);
        ...
    }

but in the _requireOnlyOperatorOrOwnerOf we are passing msg.sender as both the owner and spender when callingERC721Upgradeable._isAuthorized

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

https://github.com/OpenZeppelin/openzeppelin-contracts-upgradeable/blob/56118009a30763d6b635676bed7968874036bb53/contracts/token/ERC721/ERC721Upgradeable.sol#L215C3-L219C6

    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);
    }

hence it will always return true since owner == spender

This can cause any user to stake a small amount and prvent the actual owner fromwithdrawing the stakedAMount, indefintely.

Impact

Anyone can add to the stakedPosition resulting the lockTime to be extended to undefined time

Code Snippet

https://github.com/OpenZeppelin/openzeppelin-contracts-upgradeable/blob/56118009a30763d6b635676bed7968874036bb53/contracts/token/ERC721/ERC721Upgradeable.sol#L215C3-L219C6 https://github.com/sherlock-audit/2024-06-magicsea/blob/42e799446595c542eff9519353d3becc50cdba63/magicsea-staking/src/MlumStaking.sol#L140-L143 https://github.com/sherlock-audit/2024-06-magicsea/blame/42e799446595c542eff9519353d3becc50cdba63/magicsea-staking/src/MlumStaking.sol#L397-L428

Tool used

Manual Review

Recommendation

Use the correct validation funciton to check the owner

Duplicate of #378

sherlock-audit / 2024-06-magicsea-judging

dhank - MlumStaking.sol :: Anyone can add to the stakedPosition resulting the lockTime of the actual owner to be extended to undefined time #653