StakingRewards: recoverERC20 should be able to withdraw excess stakingTokens

[code423n4]

Lines of code

https://github.com/code-423n4/2022-09-y2k-finance/blob/ac3e86f07bc2f1f51148d2265cc897e8b494adf7/src/rewards/StakingRewards.sol#L213-L223

Vulnerability details

Impact

The recoverERC20 function of the StakingRewards contract cannot withdraw stakingTokens, however, since the stakingTokens deposited by the user are already recorded using the _totalSupply variable, the recoverERC20 function should be able to withdraw stakingTokens exceeding _totalSupply if the user mistakenly transfers stakingTokens to the contract or the reward includes stakingToken.

Proof of Concept

https://github.com/code-423n4/2022-09-y2k-finance/blob/ac3e86f07bc2f1f51148d2265cc897e8b494adf7/src/rewards/StakingRewards.sol#L213-L223

Tools Used

None

Recommended Mitigation Steps

Change to

    function recoverERC20(address tokenAddress, uint256 tokenAmount)
        external
        onlyOwner
    {
        ERC20(tokenAddress).safeTransfer(owner, tokenAmount);
        require(ERC20(tokenAddress).balanceOf(this) >= _totalSupply);
        emit Recovered(tokenAddress, tokenAmount);
    }

[HickupHH3]

Worthwhile consideration, but disagree with severity. Funds directly transferred should be considered to be lost; it's not the protocol's responsibility to have recovery methods.

Part of #51

[HickupHH3]

I forgot that stakingToken is ERC1155, issue is therefore invalid.