From 13c2553fc6cb92e71e064650fe42ba725f9b5ca1 Mon Sep 17 00:00:00 2001
From: Fangting Liu <fangting@alchemy.com>
Date: Thu, 14 Dec 2023 17:27:01 -0800
Subject: [PATCH] fix: [spearbit-94] runtime validation bypass special to only
 allow installPlugin and upgradeToAndCall

---
 src/account/UpgradeableModularAccount.sol     |  4 +--
 ...gradeableModularAccountPluginManager.t.sol | 35 +++++++++++++++++++
 2 files changed, 37 insertions(+), 2 deletions(-)

diff --git a/src/account/UpgradeableModularAccount.sol b/src/account/UpgradeableModularAccount.sol
index ffe71598b..1fd6e3c2d 100644
--- a/src/account/UpgradeableModularAccount.sol
+++ b/src/account/UpgradeableModularAccount.sol
@@ -619,8 +619,8 @@ contract UpgradeableModularAccount is
                         && (
                             (
                                 msg.sig != IPluginManager.installPlugin.selector
-                                    || msg.sig != UUPSUpgradeable.upgradeToAndCall.selector
-                            ) && msg.sender != address(this)
+                                    && msg.sig != UUPSUpgradeable.upgradeToAndCall.selector
+                            ) || msg.sender != address(this)
                         )
                 ) {
                     // Runtime calls cannot be made against functions with no
diff --git a/test/account/UpgradeableModularAccountPluginManager.t.sol b/test/account/UpgradeableModularAccountPluginManager.t.sol
index 70b65a9df..018743ef5 100644
--- a/test/account/UpgradeableModularAccountPluginManager.t.sol
+++ b/test/account/UpgradeableModularAccountPluginManager.t.sol
@@ -858,6 +858,41 @@ contract UpgradeableModularAccountPluginManagerTest is Test {
         IStandardExecutor(account2).executeBatch(calls);
     }
 
+    function test_uninstallAndInstallInBatch_failwithOtherCalls() external {
+        // Check that we can uninstall the `MultiOwnerPlugin`, leaving no
+        // validator on `installPlugin`, and then install a different plugin
+        // immediately after as part of the same batch execution. This is a
+        // special case: normally an execution function with no runtime
+        // validator cannot be runtime-called.
+
+        // Here we test only the above is allowed, any other self-call is blocked
+
+        vm.startPrank(owner2);
+
+        ComprehensivePlugin plugin = new ComprehensivePlugin();
+        bytes32 manifestHash = keccak256(abi.encode(plugin.pluginManifest()));
+
+        Call[] memory calls = new Call[](2);
+        calls[0] = Call({
+            target: address(account2),
+            value: 0,
+            data: abi.encodeCall(IPluginManager.uninstallPlugin, (address(multiOwnerPlugin), "", "", new bytes[](0)))
+        });
+        calls[1] = Call({
+            target: address(account2),
+            value: 0,
+            data: abi.encodeCall(UpgradeableModularAccount.execute, (ethRecipient, 1 wei, ""))
+        });
+
+        vm.expectRevert(
+            abi.encodeWithSelector(
+                UpgradeableModularAccount.RuntimeValidationFunctionMissing.selector,
+                UpgradeableModularAccount.execute.selector
+            )
+        );
+        IStandardExecutor(account2).executeBatch(calls);
+    }
+
     function test_noNonSelfInstallAfterUninstall() external {
         // A companion to the previous test, ensuring that `installPlugin` can't
         // be called directly (e.g. not via `execute` or `executeBatch`) if it