Unpredictable behavior for users due to admin front running or general bad timing
In a number of cases, administrators of contracts can update or upgrade things in the system without warning. This has the potential to violate a security goal of the system.\\nSpecifically, privileged roles could use front running to make malicious changes just ahead of incoming transactions, or purely accidental negative effects could occur due to the unfortunate timing of changes.\\nIn general users of the system should have assurances about the behavior of the action they're about to take.\\nMooniswapFactoryGovernance - Admin opportunity to lock `swapFor` with a referral when setting an invalid `referralFeeReceiver`\\n`setReferralFeeReceiver` and `setGovernanceFeeReceiver` takes effect immediately.\\n```\\nfunction setReferralFeeReceiver(address newReferralFeeReceiver) external onlyOwner {\\n    referralFeeReceiver = newReferralFeeReceiver;\\n    emit ReferralFeeReceiverUpdate(newReferralFeeReceiver);\\n}\\n```\\n\\n`setReferralFeeReceiver` can be used to set an invalid receiver address (or one that reverts on every call) effectively rendering `Mooniswap.swapFor` unusable if a referral was specified in the swap.\\n```\\nif (referral != address(0)) {\\n    referralShare = invIncrease.mul(referralShare).div(\\_FEE\\_DENOMINATOR);\\n    if (referralShare > 0) {\\n        if (referralFeeReceiver != address(0)) {\\n            \\_mint(referralFeeReceiver, referralShare);\\n            IReferralFeeReceiver(referralFeeReceiver).updateReward(referral, referralShare);\\n```\\n\\nLocking staked token\\nAt any point in time and without prior notice to users an admin may accidentally or intentionally add a broken governance sub-module to the system that blocks all users from unstaking their `1INCH` token. An admin can recover from this by removing the broken sub-module, however, with malicious intent tokens may be locked forever.\\nSince `1INCH` token gives voting power in the system, tokens are considered to hold value for other users and may be traded on exchanges. This raises concerns if tokens can be locked in a contract by one actor.\\nAn admin adds an invalid address or a malicious sub-module to the governance contract that always `reverts` on calls to `notifyStakeChanged`.\\n```\\nfunction addModule(address module) external onlyOwner {\\n    require(\\_modules.add(module), "Module already registered");\\n    emit AddModule(module);\\n}\\n```\\n\\n```\\nfunction \\_notifyFor(address account, uint256 balance) private {\\n    uint256 modulesLength = \\_modules.length();\\n    for (uint256 i = 0; i < modulesLength; ++i) {\\n        IGovernanceModule(\\_modules.at(i)).notifyStakeChanged(account, balance);\\n    }\\n}\\n```\\n\\nAdmin front-running to prevent user stake sync\\nAn admin may front-run users while staking in an attempt to prevent submodules from being notified of the stake update. This is unlikely to happen as it incurs costs for the attacker (front-back-running) to normal users but may be an interesting attack scenario to exclude a whale's stake from voting.\\nFor example, an admin may front-run `stake()` or `notoify*()` by briefly removing all governance submodules from the mothership and re-adding them after the users call succeeded. The stake-update will not be propagated to the sub-modules. A user may only detect this when they are voting (if they had no stake before) or when they actually check their stake. Such an attack might likely stay unnoticed unless someone listens for `addmodule` `removemodule` events on the contract.\\nAn admin front-runs a transaction by removing all modules and re-adding them afterwards to prevent the stake from propagating to the submodules.\\n```\\nfunction removeModule(address module) external onlyOwner {\\n    require(\\_modules.remove(module), "Module was not registered");\\n    emit RemoveModule(module);\\n}\\n```\\n\\nAdmin front-running to prevent unstake from propagating\\nAn admin may choose to front-run their own `unstake()`, temporarily removing all governance sub-modules, preventing `unstake()` from syncing the action to sub-modules while still getting their previously staked tokens out. The governance sub-modules can be re-added right after unstaking. Due to double-accounting of the stake (in governance and in every sub-module) their stake will still be exercisable in the sub-module even though it was removed from the mothership. Users can only prevent this by manually calling a state-sync on the affected account(s).
The underlying issue is that users of the system can't be sure what the behavior of a function call will be, and this is because the behavior can change at any time.\\nWe recommend giving the user advance notice of changes with a time lock. For example, make all system-parameter and upgrades require two steps with a mandatory time window between them. The first step merely broadcasts to users that a particular change is coming, and the second step commits that change after a suitable waiting period. This allows users that do not accept the change to withdraw immediately.\\nFurthermore, users should be guaranteed to be able to redeem their staked tokens. An entity - even though trusted - in the system should not be able to lock tokens indefinitely.
```\\nfunction setReferralFeeReceiver(address newReferralFeeReceiver) external onlyOwner {\\n    referralFeeReceiver = newReferralFeeReceiver;\\n    emit ReferralFeeReceiverUpdate(newReferralFeeReceiver);\\n}\\n```\\n