From 3fb4725c871a2c6f368dbd82a87a1fc0c5658902 Mon Sep 17 00:00:00 2001 From: Rainer Buschow Date: Mon, 18 May 2026 14:01:42 +0200 Subject: [PATCH] Fix DotNetHook: produce consistent signature and attributes for hook stubs When hooking an instance method, the generated dummy stub on reused callingMethod.Signature (which carries HasThis=true) while forcing IsStatic=true via an object initializer. AsmResolver rejects the resulting metadata with: "Method is static but its signature has the HasThis flag set." Under .NET 10 / ASP.NET Core, where the protection touches hundreds of call sites in real-world assemblies, this surfaces as 500+ WriteModule errors and the obfuscated module is never written. This change: 1. Rebuilds the stub signature as truly static via MethodSignature.CreateStatic. For instance methods, the declaring type is prepended as a regular first parameter so the IL call stack at the caller stays the same. 2. Sets MethodAttributes.Assembly | MethodAttributes.Static directly in the MethodDefinition constructor (after stripping the inherited visibility/static-related bits). Setting IsStatic=true later via an object initializer was too late, because AsmResolver verifies the attributes against the signature inside the constructor, throwing: "An instance method requires a signature with the HasThis flag set." 3. Adds an explicit "this" ParameterDefinition for the prepended slot and shifts the sequence numbers of the cloned parameters by one. 4. Skips generic methods. The cloned signature has GenericParameterCount > 0 but no GenericParameter definitions are attached to the stub, which AsmResolver rejects with: "Method defines 0 generic parameters but its signature defines N parameters." Fully cloning the generic parameters (including their constraints) is non-trivial, so we conservatively skip these call sites. --- src/BitMono.Protections/DotNetHook.cs | 74 +++++++++++++++++++++++++-- 1 file changed, 69 insertions(+), 5 deletions(-) diff --git a/src/BitMono.Protections/DotNetHook.cs b/src/BitMono.Protections/DotNetHook.cs index b8e85dc4..01f1f261 100644 --- a/src/BitMono.Protections/DotNetHook.cs +++ b/src/BitMono.Protections/DotNetHook.cs @@ -59,20 +59,84 @@ public override Task ExecuteAsync() { continue; } + // Skip generic methods. The cloned signature carries + // GenericParameterCount > 0, but we would not attach + // corresponding GenericParameter definitions to the + // generated dummyMethod. AsmResolver then rejects the + // assembly during WriteModule with: + // "Method defines 0 generic parameters but its signature + // defines N parameters." + // Fully cloning the GenericParameters (including their + // constraints) is non-trivial, so we conservatively skip + // these calls. + if (callingMethod.Signature.GenericParameterCount > 0) + { + continue; + } if (module.TryLookupMember(callingMethod.MetadataToken, out var callingMethodMetadata) == false) { continue; } - var dummyMethod = new MethodDefinition(_renamer.RenameUnsafely(), callingMethod.Attributes, callingMethod.Signature) + // If the original method is an instance method, its signature + // carries the HasThis flag. Previously callingMethod.Signature + // was reused by reference while the dummy method was forced to + // IsStatic = true, which produced inconsistent metadata + // ("Method is static but its signature has the HasThis flag + // set."). Under .NET 10 / ASP.NET Core this triggers hundreds + // of WriteModuleAsync errors. We now rebuild the signature + // explicitly as static, prepending "this" as a regular first + // parameter so the IL call stack at the caller stays the same. + var originalSignature = callingMethod.Signature; + MethodSignature dummySignature; + bool prependThis = originalSignature.HasThis; + if (prependThis) + { + var paramTypes = new List(originalSignature.ParameterTypes.Count + 1) + { + callingMethod.DeclaringType!.ToTypeSignature() + }; + paramTypes.AddRange(originalSignature.ParameterTypes); + dummySignature = MethodSignature.CreateStatic( + originalSignature.ReturnType, + originalSignature.GenericParameterCount, + paramTypes.ToArray()); + } + else { - IsAssembly = true, - IsStatic = true - }.AssignNextAvailableToken(module); + dummySignature = originalSignature; + } + + // The attributes have to be correct at construction time + // because AsmResolver verifies them against the signature + // inside MethodDefinition's constructor. Setting + // "IsStatic = true" later via an object initializer is too + // late and would throw "An instance method requires a + // signature with the HasThis flag set." Strip all + // visibility/static-related flags from the original method + // and set Assembly + Static directly. + const MethodAttributes visibilityMask = + MethodAttributes.MemberAccessMask | + MethodAttributes.Static | + MethodAttributes.Virtual | + MethodAttributes.Final | + MethodAttributes.Abstract | + MethodAttributes.NewSlot; + var dummyAttributes = (callingMethod.Attributes & ~visibilityMask) + | MethodAttributes.Assembly + | MethodAttributes.Static; + + var dummyMethod = new MethodDefinition(_renamer.RenameUnsafely(), dummyAttributes, dummySignature) + .AssignNextAvailableToken(module); moduleType.Methods.Add(dummyMethod); + if (prependThis) + { + dummyMethod.ParameterDefinitions.Add(new ParameterDefinition(1, "this", 0)); + } foreach (var actualParameter in callingMethod.ParameterDefinitions) { - var parameter = new ParameterDefinition(actualParameter.Sequence, + var parameter = new ParameterDefinition( + prependThis ? (ushort)(actualParameter.Sequence + 1) : actualParameter.Sequence, actualParameter.Name, actualParameter.Attributes); dummyMethod.ParameterDefinitions.Add(parameter); }