Bitter Seaweed Eagle - `MlumStaking::harvestPositionsTo` can only be called by the token owner

[sherlock-admin2]

Bitter Seaweed Eagle

Low/Info

MlumStaking::harvestPositionsTo can only be called by the token owner

Summary

MlumStaking::harvestPositionsTo is intended to do (as per code comments) ` /**

• @dev Harvest from multiple staking positions to "to" address
• Can only be called by lsNFT's owner or approved address */ ` But cannot be called by anyone else but the owner of the token.

Vulnerability Detail

When calling MlumStaking::harvestPositionsTo the first authentication is _requireOnlyApprovedOrOwnerOf(tokenId) which passes if the caller is the owner or someone who has been approved aligning with the purpose of the harvestPositionsTo function. But the second check which is

require(
                (msg.sender == tokenOwner && msg.sender == to),
                "FORBIDDEN"
            );

will not pass if the caller is an approved address, making the function unable to be called from an approved address.

Impact

Broken assumptions of a function. POC

    function testHarvestTo() public{
        _stakingToken.mint(ALICE, 2 ether);
        vm.startPrank(ALICE);
        _stakingToken.approve(address(_pool), 2 ether);
        _pool.createPosition(1 ether,1 days);
        _pool.createPosition(1 ether,1 days);
        ERC721(address(_pool)).approve(BOB, 1);
        ERC721(address(_pool)).approve(BOB, 2);
        vm.stopPrank();

        uint256[2] memory ids=[uint256(1),uint256(2)];
        vm.startPrank(BOB);
        vm.expectRevert();
        _pool.harvestPositionsTo(ids,ALICE);
        vm.stopPrank();
    }

Code Snippet

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

Tool used

Manual Review VSCode Foundry

Recommendation

Remove the require statement as all of the checks necessary have been done by _requireOnlyApprovedOrOwnerOf(tokenId);