instantUnstake function can be frontrunned with fee increase

[code423n4]

Lines of code

https://github.com/code-423n4/2022-06-yieldy/blob/main/src/contracts/LiquidityReserve.sol#L92-L98

Vulnerability details

Impact

instantUnstake() allows user to unstake their stakingToken for a fee paid to the liquidity providers. This fee could be changed up to 100% any moment by admin.

Malicious admin could frontrun users instantUnstake() transaction and set fee to any value (using setFee()) and get all users unstaking asset.

It's even could lead to a situation when non-malicious admin accidentally frontrun unstaking user by increasing fee to a new rate, which user wasn't expected.

    /**
        @notice sets Fee (in basis points eg. 100 bps = 1%) for instant unstaking
        @param _fee uint - fee in basis points
     */
    function setFee(uint256 _fee) external onlyOwner {
        // check range before setting fee
        require(_fee <= BASIS_POINTS, "Out of range");
        fee = _fee;

        emit FeeChanged(_fee);
    }

Recommended Mitigation Steps

Consider introducing an upper limit for fees so users can know the maximum fess available in protocol and adding timelock to change fee size. This way, frontrunnig will be impossible, and users will know which fee they agree to.

[JasoonS]

Checks should be in place for this. Saying the code is upgradeable isn't an excuse for not having sanity checks in admin functions in the code.

For example script could have a bug that sets this value wrong (for example making it 1e18 times bigger than it should be or something).