fix: [quantstamp-16] Disallow using dependencies for hooks

Allowing hooks to be dependencies can lead to hooks being called in
unexpected contexts, such as state-changing validation hooks being
called outside of validation or one of a pre- and post-exec hook pair
being called without the other. To prevent this without changing the
interface from the spec, we disallow this case entirely.

src/account/PluginManagerInternals.sol

@@ -479,7 +479,7 @@ abstract contract PluginManagerInternals is IPluginManager, AccountStorageV1 {
             _addUserOpValidationFunction(
                 mv.executionSelector,
                 _resolveManifestFunction(
-                    mv.associatedFunction, plugin, dependencies, ManifestAssociatedFunctionType.NONE
+                    mv.associatedFunction, plugin, dependencies, true, ManifestAssociatedFunctionType.NONE
                 )
             );
 
@@ -498,6 +498,7 @@ abstract contract PluginManagerInternals is IPluginManager, AccountStorageV1 {
                     mv.associatedFunction,
                     plugin,
                     dependencies,
+                    true,
                     ManifestAssociatedFunctionType.RUNTIME_VALIDATION_ALWAYS_ALLOW
                 )
             );
@@ -517,6 +518,7 @@ abstract contract PluginManagerInternals is IPluginManager, AccountStorageV1 {
                     mh.associatedFunction,
                     plugin,
                     dependencies,
+                    false,
                     ManifestAssociatedFunctionType.PRE_HOOK_ALWAYS_DENY
                 )
             );
@@ -536,6 +538,7 @@ abstract contract PluginManagerInternals is IPluginManager, AccountStorageV1 {
                     mh.associatedFunction,
                     plugin,
                     dependencies,
+                    false,
                     ManifestAssociatedFunctionType.PRE_HOOK_ALWAYS_DENY
                 )
             );
@@ -551,10 +554,14 @@ abstract contract PluginManagerInternals is IPluginManager, AccountStorageV1 {
             _addExecHooks(
                 mh.executionSelector,
                 _resolveManifestFunction(
-                    mh.preExecHook, plugin, dependencies, ManifestAssociatedFunctionType.PRE_HOOK_ALWAYS_DENY
+                    mh.preExecHook,
+                    plugin,
+                    dependencies,
+                    false,
+                    ManifestAssociatedFunctionType.PRE_HOOK_ALWAYS_DENY
                 ),
                 _resolveManifestFunction(
-                    mh.postExecHook, plugin, dependencies, ManifestAssociatedFunctionType.NONE
+                    mh.postExecHook, plugin, dependencies, false, ManifestAssociatedFunctionType.NONE
                 )
             );
 
@@ -573,12 +580,14 @@ abstract contract PluginManagerInternals is IPluginManager, AccountStorageV1 {
                     manifest.permittedCallHooks[i].preExecHook,
                     plugin,
                     dependencies,
+                    false,
                     ManifestAssociatedFunctionType.PRE_HOOK_ALWAYS_DENY
                 ),
                 _resolveManifestFunction(
                     manifest.permittedCallHooks[i].postExecHook,
                     plugin,
                     dependencies,
+                    false,
                     ManifestAssociatedFunctionType.NONE
                 )
             );
@@ -695,12 +704,14 @@ abstract contract PluginManagerInternals is IPluginManager, AccountStorageV1 {
                     args.manifest.permittedCallHooks[i].preExecHook,
                     args.plugin,
                     dependencies,
+                    false,
                     ManifestAssociatedFunctionType.PRE_HOOK_ALWAYS_DENY
                 ),
                 _resolveManifestFunction(
                     args.manifest.permittedCallHooks[i].postExecHook,
                     args.plugin,
                     dependencies,
+                    false,
                     ManifestAssociatedFunctionType.NONE
                 )
             );
@@ -717,10 +728,14 @@ abstract contract PluginManagerInternals is IPluginManager, AccountStorageV1 {
             _removeExecHooks(
                 mh.executionSelector,
                 _resolveManifestFunction(
-                    mh.preExecHook, args.plugin, dependencies, ManifestAssociatedFunctionType.PRE_HOOK_ALWAYS_DENY
+                    mh.preExecHook,
+                    args.plugin,
+                    dependencies,
+                    false,
+                    ManifestAssociatedFunctionType.PRE_HOOK_ALWAYS_DENY
                 ),
                 _resolveManifestFunction(
-                    mh.postExecHook, args.plugin, dependencies, ManifestAssociatedFunctionType.NONE
+                    mh.postExecHook, args.plugin, dependencies, false, ManifestAssociatedFunctionType.NONE
                 )
             );
 
@@ -740,6 +755,7 @@ abstract contract PluginManagerInternals is IPluginManager, AccountStorageV1 {
                     mh.associatedFunction,
                     args.plugin,
                     dependencies,
+                    false,
                     ManifestAssociatedFunctionType.PRE_HOOK_ALWAYS_DENY
                 )
             );
@@ -760,6 +776,7 @@ abstract contract PluginManagerInternals is IPluginManager, AccountStorageV1 {
                     mh.associatedFunction,
                     args.plugin,
                     dependencies,
+                    false,
                     ManifestAssociatedFunctionType.PRE_HOOK_ALWAYS_DENY
                 )
             );
@@ -939,13 +956,18 @@ abstract contract PluginManagerInternals is IPluginManager, AccountStorageV1 {
         ManifestFunction memory manifestFunction,
         address plugin,
         FunctionReference[] memory dependencies,
+        bool allowDependencyType,
         // Indicates which magic value, if any, is permissible for the function to resolve.
         ManifestAssociatedFunctionType allowedMagicValue
     ) internal pure returns (FunctionReference) {
         if (manifestFunction.functionType == ManifestAssociatedFunctionType.SELF) {
             return FunctionReferenceLib.pack(plugin, manifestFunction.functionId);
         } else if (manifestFunction.functionType == ManifestAssociatedFunctionType.DEPENDENCY) {
-            return dependencies[manifestFunction.dependencyIndex];
+            if (allowDependencyType) {
+                return dependencies[manifestFunction.dependencyIndex];
+            } else {
+                revert InvalidPluginManifest();
+            }
         } else if (manifestFunction.functionType == ManifestAssociatedFunctionType.RUNTIME_VALIDATION_ALWAYS_ALLOW)
         {
             if (allowedMagicValue == ManifestAssociatedFunctionType.RUNTIME_VALIDATION_ALWAYS_ALLOW) {

src/interfaces/IPlugin.sol

@@ -58,7 +58,7 @@ struct ManifestExternalCallPermission {
 struct PluginManifest {
     // List of ERC-165 interfaceIds to add to account to support introspection checks.
     bytes4[] interfaceIds;
-    // If this plugin depends on other plugins' validation functions and/or hooks, the interface IDs of
+    // If this plugin depends on other plugins' validation functions, the interface IDs of
     // those plugins MUST be provided here, with its position in the array matching the `dependencyIndex`
     // members of `ManifestFunction` structs used in the manifest.
     bytes4[] dependencyInterfaceIds;