t.aksoy - Invalid authorized check for staking tokens

[sherlock-admin4]

t.aksoy

Medium

Invalid authorized check for staking tokens

Summary

The authorization check for operators or owners in the MlumStaking::addToPosition() function always returns true, allowing any user to access this function for any token ID.

Vulnerability Detail

The MlumStaking::addToPosition() function is intended to be callable only by the owner or operators. It includes a check using _requireOnlyOperatorOrOwnerOf(), which uses msg.sender as both the owner and spender. This logic flaw results in the check always returning true, therefore allowing any user to call addToPosition() for any token ID.

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

Impact

MlumStaking::addToPosition() can be called by any user to lock another users token further

Code Snippet

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

Tool used

Manual Review

Recommendation

Modify the check to use the actual token owner instead of msg.sender:

       require(ERC721Upgradeable._isAuthorized(owner, msg.sender, tokenId), "FORBIDDEN");

Duplicate of #378