Check if plugin supports expected interface Id when enabling

contracts/SafeProtocolManager.sol

@@ -180,6 +180,11 @@ contract SafeProtocolManager is ISafeProtocolManager, RegistryManager, HooksMana
         address plugin,
         bool allowRootAccess
     ) external noZeroOrSentinelPlugin(plugin) onlyPermittedIntegration(plugin) onlyAccount {
+        // address(0) check omitted because it is not expected to enable it as a plugin and
+        // call to it would fail. Additionally, registry should not permit address(0) as an integration.
+        if (!ISafeProtocolPlugin(plugin).supportsInterface(type(ISafeProtocolPlugin).interfaceId))
+            revert AccountDoesNotImplementValidInterfaceId(plugin);
+
         PluginAccessInfo storage senderSentinelPlugin = enabledPlugins[msg.sender][SENTINEL_MODULES];
         PluginAccessInfo storage senderPlugin = enabledPlugins[msg.sender][plugin];
 

test/SafeProtocolManager.spec.ts

@@ -94,6 +94,39 @@ describe("SafeProtocolManager", async () => {
                     .withArgs(pluginAddress, 0, 0);
             });
 
+            it("Should not allow a Safe to enable plugin that does not support ERC165", async () => {
+                const { safeProtocolManager, safe, safeProtocolRegistry } = await loadFixture(deployContractsWithPluginFixture);
+                await safe.setModule(await safeProtocolManager.getAddress());
+
+                const mockPlugin = await hre.ethers.deployContract("MockContract");
+                await mockPlugin.givenMethodReturnBool("0x01ffc9a7", true);
+                await safeProtocolRegistry.connect(owner).addIntegration(mockPlugin.target, IntegrationType.Plugin);
+
+                await mockPlugin.givenMethodReturnBool("0x01ffc9a7", false);
+                const data = safeProtocolManager.interface.encodeFunctionData("enablePlugin", [mockPlugin.target, false]);
+                await expect(safe.exec(safe.target, 0, data))
+                    .to.be.revertedWithCustomError(safeProtocolManager, "AccountDoesNotImplementValidInterfaceId")
+                    .withArgs(mockPlugin.target);
+            });
+
+            it("Should not allow a Safe to enable plugin having no code", async () => {
+                // This test simulates case when a Plugin is added in registry but later has no code
+                // because it calls selfdestruct.
+
+                // Setup mock registry. Required to bypass ERC165 checks for user2.address as it is an EOA.
+                const safeProtocolRegistry = await hre.ethers.deployContract("MockContract");
+                await safeProtocolRegistry.givenMethodReturnBool("0x01ffc9a7", true);
+
+                const safeProtocolManager = await (
+                    await hre.ethers.getContractFactory("SafeProtocolManager")
+                ).deploy(owner.address, await safeProtocolRegistry.getAddress());
+
+                const safe = await hre.ethers.deployContract("TestExecutor", [safeProtocolManager.target], { signer: deployer });
+                const data = safeProtocolManager.interface.encodeFunctionData("enablePlugin", [user2.address, false]);
+
+                await expect(safe.exec(safe.target, 0, data)).to.be.reverted;
+            });
+
             it("Should not allow a Safe to enable plugin if flagged in registry", async () => {
                 const { safeProtocolManager, safe, plugin, safeProtocolRegistry } = await loadFixture(deployContractsWithPluginFixture);
                 await safe.setModule(await safeProtocolManager.getAddress());