FlywheelCore's setFlywheelRewards can remove access to reward funds from current users

[code423n4]

Lines of code

https://github.com/fei-protocol/flywheel-v2/blob/77bfadf388db25cf5917d39cd9c0ad920f404aad/src/FlywheelCore.sol#L164-L171

Vulnerability details

Impact

FlywheelCore.setFlywheelRewards can remove current reward funds from the current users' reach as it doesn't check that newFlywheelRewards' FlywheelCore is this contract.

If it's not, by mistake or with a malicious intent, the users will lose the access to reward funds as this FlywheelCore will not be approved for any fund access to the new flywheelRewards, while all the reward funds be moved there.

Setting severity to medium as on one hand that's system breaking issue (no rewards can be claimed after that, users are rugged reward-wise), on the other hand setFlywheelRewards function is requiresAuth. Also, a room for operational mistake isn't too small here as new flywheelRewards contract can be correctly configured and not malicious in all other regards.

Proof of Concept

FlywheelCore.setFlywheelRewards doesn't check that newFlywheelRewards' FlywheelCore is this FlywheelCore instance:

https://github.com/fei-protocol/flywheel-v2/blob/77bfadf388db25cf5917d39cd9c0ad920f404aad/src/FlywheelCore.sol#L164-L171

FlywheelCore is immutable within flywheelRewards and its access to the flywheelRewards' funds is set on construction:

https://github.com/fei-protocol/flywheel-v2/blob/77bfadf388db25cf5917d39cd9c0ad920f404aad/src/rewards/BaseFlywheelRewards.sol#L30

This way if new flywheelRewards contract have any different FlywheelCore then current users' access to reward funds will be irrevocably lost as both claiming functionality and next run of setFlywheelRewards will revert, not being able to transfer any funds from flywheelRewards with rewardToken.safeTransferFrom(address(flywheelRewards), ...):

https://github.com/fei-protocol/flywheel-v2/blob/77bfadf388db25cf5917d39cd9c0ad920f404aad/src/FlywheelCore.sol#L125

https://github.com/fei-protocol/flywheel-v2/blob/77bfadf388db25cf5917d39cd9c0ad920f404aad/src/FlywheelCore.sol#L168

As FlywheelCore holds user funds accounting via rewardsAccrued mapping, all these accounts became non-operational, as all the unclaimed rewards will be lost for the users.

Recommended Mitigation Steps

Consider adding the require for address(newFlywheelRewards.flywheel) == address(flywheelRewards.flywheel) in setFlywheelRewards so that users always retain funds access.

[Joeysantoro]

Similar to #23 . I think adding this check makes sense

[0xean]

This issue seems distinct enough from #23 to warrant seperate issues. Leaving open and not as a dupe.