Make callback function calls nonReentrant

src/ModularCore.sol

@@ -10,8 +10,9 @@ import {IInstallationCallback} from "./interface/IInstallationCallback.sol";
 import {Role} from "./Role.sol";
 import {OwnableRoles} from "@solady/auth/OwnableRoles.sol";
 import {EnumerableSetLib} from "@solady/utils/EnumerableSetLib.sol";
+import {ReentrancyGuard} from "@solady/utils/ReentrancyGuard.sol";
 
-abstract contract ModularCore is IModularCore, OwnableRoles {
+abstract contract ModularCore is IModularCore, OwnableRoles, ReentrancyGuard {
     using EnumerableSetLib for EnumerableSetLib.AddressSet;
 
     /*//////////////////////////////////////////////////////////////
@@ -280,6 +281,7 @@ abstract contract ModularCore is IModularCore, OwnableRoles {
     /// @dev Calls an extension callback function and checks whether it is optional or required.
     function _executeCallbackFunction(bytes4 _selector, bytes memory _abiEncodedCalldata)
         internal
+        nonReentrant
         returns (bool success, bytes memory returndata)
     {
         InstalledFunction memory callbackFunction = functionData_[_selector];

src/core/token/ERC20Core.sol

@@ -5,6 +5,7 @@ import {ModularCore} from "../../ModularCore.sol";
 
 import {ERC20} from "@solady/tokens/ERC20.sol";
 import {Multicallable} from "@solady/utils/Multicallable.sol";
+import {ReentrancyGuard} from "@solady/utils/ReentrancyGuard.sol";
 
 import {IERC20} from "../../interface/IERC20.sol";
 

test/ModularCore.t.sol

@@ -9,6 +9,7 @@ import {IExtensionConfig} from "src/interface/IExtensionConfig.sol";
 import {IModularCore} from "src/interface/IModularCore.sol";
 import {ModularExtension} from "src/ModularExtension.sol";
 import {ModularCore} from "src/ModularCore.sol";
+import {ReentrancyGuard} from "@solady/utils/ReentrancyGuard.sol";
 
 contract MockBase {
     uint256 internal constant NUMBER_OF_CALLBACK = 10;
@@ -101,7 +102,7 @@ contract MockExtensionWithFunctions is MockBase, ModularExtension {
         return number;
     }
 
-    function callbackFunctionOne() external {
+    function callbackFunctionOne() external virtual {
         emit CallbackFunctionOne();
     }
 
@@ -120,6 +121,12 @@ contract MockExtensionWithFunctions is MockBase, ModularExtension {
     function permissioned_staticcall() external view {}
 }
 
+contract MockExtensionReentrant is MockExtensionWithFunctions {
+    function callbackFunctionOne() external override {
+        MockCore(payable(address(this))).callbackFunctionOne();
+    }
+}
+
 contract MockExtensionOnInstallFails is MockExtensionWithFunctions {
     error OnInstallFailed();
 
@@ -342,6 +349,17 @@ contract ModularCoreTest is Test {
         MockExtensionWithFunctions(address(core)).notPermissioned_call();
     }
 
+    function test_installExtension_revert_reentrantCallbackFunction() public {
+        MockExtensionReentrant ext = new MockExtensionReentrant();
+
+        // Install extension
+        vm.prank(owner);
+        core.installExtension(address(ext), "");
+
+        vm.expectRevert(abi.encodeWithSelector(ReentrancyGuard.Reentrancy.selector));
+        core.callbackFunctionOne();
+    }
+
     function test_installExtension_revert_unauthorizedCaller() public {
         vm.expectRevert(abi.encodeWithSelector(0x82b42900)); // OwnableRoles.Unauthorized()
         vm.prank(unpermissionedActor);