⚡️ Adding msg.sender separation for factory calls

src/IRegistry.sol

@@ -250,7 +250,9 @@ interface IRegistry is IERC7484 {
     /**
      * In order to make the integration into existing business logics possible,
      * the Registry is able to utilize external factories that can be utilized to deploy the modules.
-     * @dev Registry can use other factories to deploy the module
+     * @dev Registry can use other factories to deploy the module.
+     * @dev Note that this function will call the external factory via the FactoryTrampoline contract.
+     *           Factory MUST not assume that msg.sender == registry
      * @dev This function is used to deploy and register a module using a factory contract.
      *           Since one of the parameters of this function is a unique resolverUID and any
      *           registered module address can only be registered once,

src/core/ModuleManager.sol

@@ -9,6 +9,32 @@ import { ResolverRecord, ModuleRecord, ResolverUID } from "../DataTypes.sol";
 import { ResolverManager } from "./ResolverManager.sol";
 import { IRegistry } from "../IRegistry.sol";
 
+/**
+ * In order to separate msg.sender context from registry,
+ * interactions with external Factories are done with this Trampoline contract.
+ */
+contract FactoryTrampoline {
+    error FactoryCallFailed(address factory);
+
+    /**
+     * @param factory the address of the factory to call
+     * @param callOnFactory the call data to send to the factory
+     * @return moduleAddress the moduleAddress that was returned by the
+     */
+    function deployViaFactory(address factory, bytes memory callOnFactory) external payable returns (address moduleAddress) {
+        // call external factory to deploy module
+        bool success;
+        /* solhint-disable no-inline-assembly */
+        assembly ("memory-safe") {
+            success := call(gas(), factory, callvalue(), add(callOnFactory, 0x20), mload(callOnFactory), 0, 32)
+            moduleAddress := mload(0)
+        }
+        if (!success) {
+            revert FactoryCallFailed(factory);
+        }
+    }
+}
+
 /**
  * In order for Attesters to be able to make statements about a Module, the Module first needs to be registered on the Registry.
  * This can be done as part of or after Module deployment. On registration, every module is tied to a
@@ -34,6 +60,12 @@ abstract contract ModuleManager is IRegistry, ResolverManager {
 
     mapping(address moduleAddress => ModuleRecord moduleRecord) internal $moduleAddrToRecords;
 
+    FactoryTrampoline private immutable FACTORY_TRAMPOLINE;
+
+    constructor() {
+        FACTORY_TRAMPOLINE = new FactoryTrampoline();
+    }
+
     /**
      * @inheritdoc IRegistry
      */
@@ -105,11 +137,9 @@ abstract contract ModuleManager is IRegistry, ResolverManager {
         // prevent someone from calling a registry function pretending its a factory
         if (factory == address(this)) revert FactoryCallFailed(factory);
 
-        // call external factory to deploy module
-        (bool ok, bytes memory returnData) = factory.call{ value: msg.value }(callOnFactory);
-        if (!ok) revert FactoryCallFailed(factory);
-
-        moduleAddress = abi.decode(returnData, (address));
+        // Call the factory via the trampoline contract. This will make sure that there is msg.sender separation
+        // Making "raw" calls to user supplied addresses could create security issues.
+        moduleAddress = FACTORY_TRAMPOLINE.deployViaFactory{ value: msg.value }({ factory: factory, callOnFactory: callOnFactory });
 
         ModuleRecord memory record = _storeModuleRecord({
             moduleAddress: moduleAddress,