From ae10daaa1f3752b6d691307051d6e92927f7d09f Mon Sep 17 00:00:00 2001 From: RandomCrocodile Date: Sat, 4 Jul 2026 14:43:16 +0200 Subject: [PATCH 1/3] feature: add Confuser.Analyzers project with CX004 analyzer (#49) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit First increment of the Tier 3 custom analyzers (#49, parent #46). Establishes the Confuser.Analyzers project (netstandard2.0 Roslyn analyzer) and wires it into every project via ConfuserEx.Common.targets as an analyzer reference. CX004 — flags Assembly.GetTypes()/Module.GetTypes() that are not guarded against ReflectionTypeLoadException (the cause of packer/plugin startup crashes). The analyzer resolves the invocation symbol and only fires for System.Reflection types, so dnlib's ModuleDef.GetTypes() (which returns TypeDef and never throws) is ignored. - Fixed the two real CX004 violations: PluginDiscovery.AddPlugins and ComponentDiscovery.LoadComponents now catch ReflectionTypeLoadException and keep the types that loaded. - Analyzer wired with SkipGetTargetFrameworkProperties + UndefineProperties so the netstandard2.0 analyzer attaches to all targets including net20. - 4 analyzer unit tests (fires on unguarded; silent when guarded by RTLE/Exception or on unrelated GetTypes()). Full solution builds clean with zero CX004 warnings. Remaining under #49 (follow-up): CX002 (unguarded Resolve*Def), CX003 (ResolveThrow audit). CX001's premise is stale — the listed call sites were refactored to a context-aware Import helper; the only host-reflection path left is that helper's fallback (Confuser.Protections/Utils.cs). --- Confuser.Analyzers/Confuser.Analyzers.csproj | 26 +++++ Confuser.Analyzers/DiagnosticIds.cs | 21 ++++ .../UnhandledReflectionTypeLoadAnalyzer.cs | 104 ++++++++++++++++++ Confuser.Core/PluginDiscovery.cs | 15 ++- Confuser2.sln | 29 +++++ ConfuserEx.Common.targets | 12 +- ConfuserEx/ComponentDiscovery.cs | 15 ++- .../Confuser.Analyzers.Test.csproj | 19 ++++ ...UnhandledReflectionTypeLoadAnalyzerTest.cs | 66 +++++++++++ 9 files changed, 302 insertions(+), 5 deletions(-) create mode 100644 Confuser.Analyzers/Confuser.Analyzers.csproj create mode 100644 Confuser.Analyzers/DiagnosticIds.cs create mode 100644 Confuser.Analyzers/UnhandledReflectionTypeLoadAnalyzer.cs create mode 100644 Tests/Confuser.Analyzers.Test/Confuser.Analyzers.Test.csproj create mode 100644 Tests/Confuser.Analyzers.Test/UnhandledReflectionTypeLoadAnalyzerTest.cs diff --git a/Confuser.Analyzers/Confuser.Analyzers.csproj b/Confuser.Analyzers/Confuser.Analyzers.csproj new file mode 100644 index 000000000..4a29d3af5 --- /dev/null +++ b/Confuser.Analyzers/Confuser.Analyzers.csproj @@ -0,0 +1,26 @@ + + + + + netstandard2.0 + false + enable + latest + true + false + + false + + $(NoWarn);RS2008 + + + + + + runtime; build; native; contentfiles; analyzers; buildtransitive + + + + diff --git a/Confuser.Analyzers/DiagnosticIds.cs b/Confuser.Analyzers/DiagnosticIds.cs new file mode 100644 index 000000000..59fbaefd6 --- /dev/null +++ b/Confuser.Analyzers/DiagnosticIds.cs @@ -0,0 +1,21 @@ +namespace Confuser.Analyzers { + /// + /// Diagnostic IDs for the ConfuserEx-specific analyzers. Each catches a bug class that has + /// actually been fixed in the codebase, to prevent regressions at compile time. + /// + internal static class DiagnosticIds { + /// Imports a member from the host runtime instead of the target's corlib. + public const string HostRuntimeImport = "CX001"; + + /// ResolveTypeDef()/ResolveMethodDef() used without a null check. + public const string UnguardedResolve = "CX002"; + + /// Usage of a ...Throw resolve helper (audit / awareness). + public const string ResolveThrowAudit = "CX003"; + + /// GetTypes() without handling ReflectionTypeLoadException. + public const string UnhandledReflectionTypeLoad = "CX004"; + + public const string Category = "ConfuserEx"; + } +} diff --git a/Confuser.Analyzers/UnhandledReflectionTypeLoadAnalyzer.cs b/Confuser.Analyzers/UnhandledReflectionTypeLoadAnalyzer.cs new file mode 100644 index 000000000..e3b2efae8 --- /dev/null +++ b/Confuser.Analyzers/UnhandledReflectionTypeLoadAnalyzer.cs @@ -0,0 +1,104 @@ +using System.Collections.Immutable; +using Microsoft.CodeAnalysis; +using Microsoft.CodeAnalysis.CSharp; +using Microsoft.CodeAnalysis.CSharp.Syntax; +using Microsoft.CodeAnalysis.Diagnostics; + +namespace Confuser.Analyzers { + /// + /// CX004 — flags Assembly.GetTypes() / Module.GetTypes() that are not guarded + /// against . That exception is + /// thrown whenever a contained type cannot be loaded (common for plugin assemblies with + /// unresolved dependencies) and caused packer/plugin startup crashes. + /// + [DiagnosticAnalyzer(LanguageNames.CSharp)] + public sealed class UnhandledReflectionTypeLoadAnalyzer : DiagnosticAnalyzer { + static readonly DiagnosticDescriptor Rule = new DiagnosticDescriptor( + DiagnosticIds.UnhandledReflectionTypeLoad, + "GetTypes() without ReflectionTypeLoadException handling", + "'{0}.GetTypes()' can throw ReflectionTypeLoadException when a contained type is unresolvable; wrap it in a try/catch", + DiagnosticIds.Category, + DiagnosticSeverity.Warning, + isEnabledByDefault: true, + description: "Assembly.GetTypes() and Module.GetTypes() throw ReflectionTypeLoadException when any " + + "contained type cannot be loaded. Guard the call (and prefer ex.Types on the exception) to avoid " + + "startup crashes when loading plugin or external assemblies."); + + public override ImmutableArray SupportedDiagnostics => ImmutableArray.Create(Rule); + + public override void Initialize(AnalysisContext context) { + context.EnableConcurrentExecution(); + context.ConfigureGeneratedCodeAnalysis(GeneratedCodeAnalysisFlags.None); + context.RegisterSyntaxNodeAction(Analyze, SyntaxKind.InvocationExpression); + } + + static void Analyze(SyntaxNodeAnalysisContext context) { + var invocation = (InvocationExpressionSyntax)context.Node; + if (invocation.Expression is not MemberAccessExpressionSyntax member || + member.Name.Identifier.ValueText != "GetTypes") + return; + + if (context.SemanticModel.GetSymbolInfo(invocation, context.CancellationToken).Symbol is not IMethodSymbol method || + method.Name != "GetTypes" || !method.Parameters.IsEmpty) + return; + + var containingType = method.ContainingType?.ToDisplayString(); + if (containingType != "System.Reflection.Assembly" && containingType != "System.Reflection.Module") + return; + + var rtle = context.Compilation.GetTypeByMetadataName("System.Reflection.ReflectionTypeLoadException"); + if (IsGuarded(invocation, context.SemanticModel, rtle, context.CancellationToken)) + return; + + context.ReportDiagnostic(Diagnostic.Create(Rule, member.Name.GetLocation(), containingType)); + } + + static bool IsGuarded(SyntaxNode node, SemanticModel model, INamedTypeSymbol? rtle, System.Threading.CancellationToken ct) { + for (var current = node.Parent; current != null; current = current.Parent) { + if (current is TryStatementSyntax tryStmt && + tryStmt.Block.Span.Contains(node.Span) && + CatchesReflectionTypeLoad(tryStmt, model, rtle, ct)) + return true; + + // Do not walk past the enclosing method / lambda / local-function boundary. + if (current is BaseMethodDeclarationSyntax || + current is AnonymousFunctionExpressionSyntax || + current is LocalFunctionStatementSyntax) + break; + } + + return false; + } + + static bool CatchesReflectionTypeLoad(TryStatementSyntax tryStmt, SemanticModel model, INamedTypeSymbol? rtle, + System.Threading.CancellationToken ct) { + foreach (var clause in tryStmt.Catches) { + // A general 'catch { }' (no declared type) catches everything. + if (clause.Declaration is null) + return true; + + var caught = model.GetTypeInfo(clause.Declaration.Type, ct).Type as INamedTypeSymbol; + if (caught is null) + continue; + + // The catch guards the call if ReflectionTypeLoadException is assignable to the caught + // type (i.e. the caught type is RTLE or one of its base types such as SystemException / + // Exception). If we cannot resolve RTLE, fall back to name matching on the base chain. + if (rtle is not null) { + for (var t = rtle; t is not null; t = t.BaseType) { + if (SymbolEqualityComparer.Default.Equals(t, caught)) + return true; + } + } + else { + var name = caught.ToDisplayString(); + if (name == "System.Exception" || name == "System.SystemException" || + name == "System.Reflection.ReflectionTypeLoadException") + return true; + } + } + + return false; + } + } +} diff --git a/Confuser.Core/PluginDiscovery.cs b/Confuser.Core/PluginDiscovery.cs index c42de92d6..158944ed9 100644 --- a/Confuser.Core/PluginDiscovery.cs +++ b/Confuser.Core/PluginDiscovery.cs @@ -55,8 +55,18 @@ public static bool HasAccessibleDefConstructor(Type type) { protected static void AddPlugins( ConfuserContext context, IList protections, IList packers, IList components, Assembly asm) { - foreach (var module in asm.GetLoadedModules()) - foreach (var i in module.GetTypes()) { + foreach (var module in asm.GetLoadedModules()) { + Type[] moduleTypes; + try { + moduleTypes = module.GetTypes(); + } + catch (ReflectionTypeLoadException ex) { + // A plugin assembly may reference dependencies that are not present; keep the + // types that did load instead of crashing plugin discovery. + moduleTypes = Array.FindAll(ex.Types, t => t != null); + } + + foreach (var i in moduleTypes) { if (i.IsAbstract || !HasAccessibleDefConstructor(i)) continue; @@ -85,6 +95,7 @@ protected static void AddPlugins( } } } + } context.CheckCancellation(); } diff --git a/Confuser2.sln b/Confuser2.sln index b200f0781..440b759d9 100644 --- a/Confuser2.sln +++ b/Confuser2.sln @@ -205,6 +205,10 @@ Project("{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}") = "CrossFramework.WPF.Net8", " EndProject Project("{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}") = "CrossFramework.Library.Net10", "Tests\CrossFramework.Library.Net10\CrossFramework.Library.Net10.csproj", "{4458415A-0F5E-4136-B723-7A67955D6047}" EndProject +Project("{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}") = "Confuser.Analyzers", "Confuser.Analyzers\Confuser.Analyzers.csproj", "{886E5FF3-DF94-4C6B-A5C3-E97038B5C275}" +EndProject +Project("{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}") = "Confuser.Analyzers.Test", "Tests\Confuser.Analyzers.Test\Confuser.Analyzers.Test.csproj", "{1B22CAAE-FC4A-478D-BD68-D29A3081F938}" +EndProject Global GlobalSection(SolutionConfigurationPlatforms) = preSolution Debug|Any CPU = Debug|Any CPU @@ -1343,6 +1347,30 @@ Global {4458415A-0F5E-4136-B723-7A67955D6047}.Release|x64.Build.0 = Release|Any CPU {4458415A-0F5E-4136-B723-7A67955D6047}.Release|x86.ActiveCfg = Release|Any CPU {4458415A-0F5E-4136-B723-7A67955D6047}.Release|x86.Build.0 = Release|Any CPU + {886E5FF3-DF94-4C6B-A5C3-E97038B5C275}.Debug|Any CPU.ActiveCfg = Debug|Any CPU + {886E5FF3-DF94-4C6B-A5C3-E97038B5C275}.Debug|Any CPU.Build.0 = Debug|Any CPU + {886E5FF3-DF94-4C6B-A5C3-E97038B5C275}.Debug|x64.ActiveCfg = Debug|Any CPU + {886E5FF3-DF94-4C6B-A5C3-E97038B5C275}.Debug|x64.Build.0 = Debug|Any CPU + {886E5FF3-DF94-4C6B-A5C3-E97038B5C275}.Debug|x86.ActiveCfg = Debug|Any CPU + {886E5FF3-DF94-4C6B-A5C3-E97038B5C275}.Debug|x86.Build.0 = Debug|Any CPU + {886E5FF3-DF94-4C6B-A5C3-E97038B5C275}.Release|Any CPU.ActiveCfg = Release|Any CPU + {886E5FF3-DF94-4C6B-A5C3-E97038B5C275}.Release|Any CPU.Build.0 = Release|Any CPU + {886E5FF3-DF94-4C6B-A5C3-E97038B5C275}.Release|x64.ActiveCfg = Release|Any CPU + {886E5FF3-DF94-4C6B-A5C3-E97038B5C275}.Release|x64.Build.0 = Release|Any CPU + {886E5FF3-DF94-4C6B-A5C3-E97038B5C275}.Release|x86.ActiveCfg = Release|Any CPU + {886E5FF3-DF94-4C6B-A5C3-E97038B5C275}.Release|x86.Build.0 = Release|Any CPU + {1B22CAAE-FC4A-478D-BD68-D29A3081F938}.Debug|Any CPU.ActiveCfg = Debug|Any CPU + {1B22CAAE-FC4A-478D-BD68-D29A3081F938}.Debug|Any CPU.Build.0 = Debug|Any CPU + {1B22CAAE-FC4A-478D-BD68-D29A3081F938}.Debug|x64.ActiveCfg = Debug|Any CPU + {1B22CAAE-FC4A-478D-BD68-D29A3081F938}.Debug|x64.Build.0 = Debug|Any CPU + {1B22CAAE-FC4A-478D-BD68-D29A3081F938}.Debug|x86.ActiveCfg = Debug|Any CPU + {1B22CAAE-FC4A-478D-BD68-D29A3081F938}.Debug|x86.Build.0 = Debug|Any CPU + {1B22CAAE-FC4A-478D-BD68-D29A3081F938}.Release|Any CPU.ActiveCfg = Release|Any CPU + {1B22CAAE-FC4A-478D-BD68-D29A3081F938}.Release|Any CPU.Build.0 = Release|Any CPU + {1B22CAAE-FC4A-478D-BD68-D29A3081F938}.Release|x64.ActiveCfg = Release|Any CPU + {1B22CAAE-FC4A-478D-BD68-D29A3081F938}.Release|x64.Build.0 = Release|Any CPU + {1B22CAAE-FC4A-478D-BD68-D29A3081F938}.Release|x86.ActiveCfg = Release|Any CPU + {1B22CAAE-FC4A-478D-BD68-D29A3081F938}.Release|x86.Build.0 = Release|Any CPU EndGlobalSection GlobalSection(SolutionProperties) = preSolution HideSolutionNode = FALSE @@ -1434,6 +1462,7 @@ Global {3942E3FD-06BC-470C-A1ED-BB18F2332B94} = {356BDB31-853E-43BB-8F9A-D8AC08F69EBB} {30DC9F52-E08A-4EF0-B041-04AED6135C3D} = {356BDB31-853E-43BB-8F9A-D8AC08F69EBB} {4458415A-0F5E-4136-B723-7A67955D6047} = {356BDB31-853E-43BB-8F9A-D8AC08F69EBB} + {1B22CAAE-FC4A-478D-BD68-D29A3081F938} = {356BDB31-853E-43BB-8F9A-D8AC08F69EBB} EndGlobalSection GlobalSection(ExtensibilityGlobals) = postSolution SolutionGuid = {0D937D9E-E04B-4A68-B639-D4260473A388} diff --git a/ConfuserEx.Common.targets b/ConfuserEx.Common.targets index c929f0088..7a9ecfccb 100644 --- a/ConfuserEx.Common.targets +++ b/ConfuserEx.Common.targets @@ -7,5 +7,15 @@ - + + + + + + + \ No newline at end of file diff --git a/ConfuserEx/ComponentDiscovery.cs b/ConfuserEx/ComponentDiscovery.cs index dc1276da0..2accc9a4e 100644 --- a/ConfuserEx/ComponentDiscovery.cs +++ b/ConfuserEx/ComponentDiscovery.cs @@ -11,8 +11,18 @@ public static void LoadComponents(IList protections, IList t != null); + } + + foreach (var i in moduleTypes) { if (i.IsAbstract || !PluginDiscovery.HasAccessibleDefConstructor(i)) continue; @@ -25,6 +35,7 @@ public static void LoadComponents(IList protections, IList + + + net10.0 + false + enable + + + + + + + + + + + + + diff --git a/Tests/Confuser.Analyzers.Test/UnhandledReflectionTypeLoadAnalyzerTest.cs b/Tests/Confuser.Analyzers.Test/UnhandledReflectionTypeLoadAnalyzerTest.cs new file mode 100644 index 000000000..06433603f --- /dev/null +++ b/Tests/Confuser.Analyzers.Test/UnhandledReflectionTypeLoadAnalyzerTest.cs @@ -0,0 +1,66 @@ +using System.Threading.Tasks; +using Microsoft.CodeAnalysis.Testing; +using Xunit; +using Verify = Microsoft.CodeAnalysis.CSharp.Testing.CSharpAnalyzerVerifier< + Confuser.Analyzers.UnhandledReflectionTypeLoadAnalyzer, + Microsoft.CodeAnalysis.Testing.DefaultVerifier>; + +namespace Confuser.Analyzers.Test { + public class UnhandledReflectionTypeLoadAnalyzerTest { + [Fact] + public async Task Flags_UnguardedAssemblyGetTypes() { + const string source = @" +using System.Reflection; +class C { + void M(Assembly asm) { + var t = asm.{|#0:GetTypes|}(); + } +}"; + var expected = Verify.Diagnostic("CX004") + .WithLocation(0) + .WithArguments("System.Reflection.Assembly"); + await Verify.VerifyAnalyzerAsync(source, expected); + } + + [Fact] + public async Task DoesNotFlag_WhenGuardedByReflectionTypeLoadException() { + const string source = @" +using System; +using System.Reflection; +class C { + void M(Assembly asm) { + try { var t = asm.GetTypes(); } + catch (ReflectionTypeLoadException) { } + } +}"; + await Verify.VerifyAnalyzerAsync(source); + } + + [Fact] + public async Task DoesNotFlag_WhenGuardedByBaseException() { + const string source = @" +using System; +using System.Reflection; +class C { + void M(Assembly asm) { + try { var t = asm.GetTypes(); } + catch (Exception) { } + } +}"; + await Verify.VerifyAnalyzerAsync(source); + } + + [Fact] + public async Task DoesNotFlag_UnrelatedGetTypesMethod() { + // A GetTypes() on a non-reflection type must not be flagged. + const string source = @" +class Other { public int[] GetTypes() => new int[0]; } +class C { + void M(Other o) { + var t = o.GetTypes(); + } +}"; + await Verify.VerifyAnalyzerAsync(source); + } + } +} From d1cf356732c30fd46345118ea25c5b49937de5d7 Mon Sep 17 00:00:00 2001 From: RandomCrocodile Date: Sat, 4 Jul 2026 14:44:46 +0200 Subject: [PATCH 2/3] feature: add CX003 Resolve...Throw audit analyzer (#49) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit CX003 surfaces every call to a dnlib Resolve...Throw helper (ResolveThrow, ResolveTypeDefThrow, ResolveMethodDefThrow, ResolveFieldThrow) at Info severity — an awareness rule for spots that crash on unresolvable references. Info severity means no build noise (the ~34 existing intentional uses are not reported as warnings). 5 tests. --- .../ResolveThrowAuditAnalyzer.cs | 54 +++++++++++++++++++ .../ResolveThrowAuditAnalyzerTest.cs | 39 ++++++++++++++ 2 files changed, 93 insertions(+) create mode 100644 Confuser.Analyzers/ResolveThrowAuditAnalyzer.cs create mode 100644 Tests/Confuser.Analyzers.Test/ResolveThrowAuditAnalyzerTest.cs diff --git a/Confuser.Analyzers/ResolveThrowAuditAnalyzer.cs b/Confuser.Analyzers/ResolveThrowAuditAnalyzer.cs new file mode 100644 index 000000000..8dab590ca --- /dev/null +++ b/Confuser.Analyzers/ResolveThrowAuditAnalyzer.cs @@ -0,0 +1,54 @@ +using System.Collections.Immutable; +using Microsoft.CodeAnalysis; +using Microsoft.CodeAnalysis.CSharp; +using Microsoft.CodeAnalysis.CSharp.Syntax; +using Microsoft.CodeAnalysis.Diagnostics; + +namespace Confuser.Analyzers { + /// + /// CX003 — an awareness/audit rule that surfaces every call to a dnlib Resolve…Throw + /// helper (ResolveThrow, ResolveTypeDefThrow, ResolveMethodDefThrow, + /// ResolveFieldThrow). These throw when a reference cannot be resolved, which crashes + /// obfuscation on assemblies with external/unresolvable members. Most uses are intentional; + /// this reports at so the spots can be evaluated + /// without adding build noise. + /// + [DiagnosticAnalyzer(LanguageNames.CSharp)] + public sealed class ResolveThrowAuditAnalyzer : DiagnosticAnalyzer { + static readonly DiagnosticDescriptor Rule = new DiagnosticDescriptor( + DiagnosticIds.ResolveThrowAudit, + "Throwing resolve helper used", + "'{0}' throws when resolution fails; confirm the target is always resolvable or prefer the non-throwing overload with a null check", + DiagnosticIds.Category, + DiagnosticSeverity.Info, + isEnabledByDefault: true, + description: "dnlib's Resolve...Throw helpers throw when a type/member reference cannot be " + + "resolved. This surfaces each usage for review; external or unresolvable references should " + + "use the non-throwing Resolve... overload with a null check instead."); + + public override ImmutableArray SupportedDiagnostics => ImmutableArray.Create(Rule); + + public override void Initialize(AnalysisContext context) { + context.EnableConcurrentExecution(); + context.ConfigureGeneratedCodeAnalysis(GeneratedCodeAnalysisFlags.None); + context.RegisterSyntaxNodeAction(Analyze, SyntaxKind.InvocationExpression); + } + + static void Analyze(SyntaxNodeAnalysisContext context) { + var invocation = (InvocationExpressionSyntax)context.Node; + if (invocation.Expression is not MemberAccessExpressionSyntax member) + return; + + var name = member.Name.Identifier.ValueText; + if (!IsResolveThrowName(name)) + return; + + context.ReportDiagnostic(Diagnostic.Create(Rule, member.Name.GetLocation(), name)); + } + + static bool IsResolveThrowName(string name) => + name.Length > "ResolveThrow".Length - 1 && + name.StartsWith("Resolve", System.StringComparison.Ordinal) && + name.EndsWith("Throw", System.StringComparison.Ordinal); + } +} diff --git a/Tests/Confuser.Analyzers.Test/ResolveThrowAuditAnalyzerTest.cs b/Tests/Confuser.Analyzers.Test/ResolveThrowAuditAnalyzerTest.cs new file mode 100644 index 000000000..8990a079f --- /dev/null +++ b/Tests/Confuser.Analyzers.Test/ResolveThrowAuditAnalyzerTest.cs @@ -0,0 +1,39 @@ +using System.Threading.Tasks; +using Microsoft.CodeAnalysis.Testing; +using Xunit; +using Verify = Microsoft.CodeAnalysis.CSharp.Testing.CSharpAnalyzerVerifier< + Confuser.Analyzers.ResolveThrowAuditAnalyzer, + Microsoft.CodeAnalysis.Testing.DefaultVerifier>; + +namespace Confuser.Analyzers.Test { + public class ResolveThrowAuditAnalyzerTest { + [Theory] + [InlineData("ResolveThrow")] + [InlineData("ResolveTypeDefThrow")] + [InlineData("ResolveMethodDefThrow")] + [InlineData("ResolveFieldThrow")] + public async Task Flags_ResolveThrowHelpers(string methodName) { + string source = @" +class Ref { public object " + methodName + @"() => null; } +class C { + void M(Ref r) { + var x = r.{|#0:" + methodName + @"|}(); + } +}"; + var expected = Verify.Diagnostic("CX003").WithLocation(0).WithArguments(methodName); + await Verify.VerifyAnalyzerAsync(source, expected); + } + + [Fact] + public async Task DoesNotFlag_NonThrowingResolve() { + const string source = @" +class Ref { public object ResolveTypeDef() => null; } +class C { + void M(Ref r) { + var x = r.ResolveTypeDef(); + } +}"; + await Verify.VerifyAnalyzerAsync(source); + } + } +} From 8afdbfa660a973cbf5efde13b519c8191a874bdd Mon Sep 17 00:00:00 2001 From: RandomCrocodile Date: Sat, 4 Jul 2026 14:54:00 +0200 Subject: [PATCH 3/3] feature: add CX002 and CX001 analyzers, completing Tier 3 (#49) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Completes the four Tier 3 analyzers. CX002 — flags a non-throwing ResolveTypeDef()/ResolveMethodDef() result that is dereferenced immediately with no null check (crashes on external/unresolvable references). Only genuine dereferences are flagged (x.ResolveTypeDef().Member / [i]); null-conditional access, assignment, return, and passing the result as an argument (e.g. to dnlib's null-tolerant SigComparer.Equals, as VTableAnalyzer does) are correctly not reported. No existing violations — pure prevention. CX001 — redesigned against the current code. The issue's original call sites were already refactored to a context-aware Import(context, Type, method) helper; the only remaining host-reflection path is that helper's last-resort fallback. CX001 now flags Import(...) whose argument is Type.GetMethod/GetConstructor/GetField/ GetProperty (host reflection → wrong-corlib reference). The single intentional fallback in Confuser.Protections/Utils.cs is isolated and suppressed with a documented pragma; new occurrences are flagged. 8 new analyzer tests (17 total). Full solution builds clean — zero CX diagnostics. --- .../HostRuntimeImportAnalyzer.cs | 64 ++++++++++++++++ .../UnguardedResolveAnalyzer.cs | 76 +++++++++++++++++++ Confuser.Protections/Utils.cs | 10 ++- .../HostRuntimeImportAnalyzerTest.cs | 63 +++++++++++++++ .../UnguardedResolveAnalyzerTest.cs | 67 ++++++++++++++++ 5 files changed, 279 insertions(+), 1 deletion(-) create mode 100644 Confuser.Analyzers/HostRuntimeImportAnalyzer.cs create mode 100644 Confuser.Analyzers/UnguardedResolveAnalyzer.cs create mode 100644 Tests/Confuser.Analyzers.Test/HostRuntimeImportAnalyzerTest.cs create mode 100644 Tests/Confuser.Analyzers.Test/UnguardedResolveAnalyzerTest.cs diff --git a/Confuser.Analyzers/HostRuntimeImportAnalyzer.cs b/Confuser.Analyzers/HostRuntimeImportAnalyzer.cs new file mode 100644 index 000000000..d9155441a --- /dev/null +++ b/Confuser.Analyzers/HostRuntimeImportAnalyzer.cs @@ -0,0 +1,64 @@ +using System.Collections.Immutable; +using Microsoft.CodeAnalysis; +using Microsoft.CodeAnalysis.CSharp; +using Microsoft.CodeAnalysis.CSharp.Syntax; +using Microsoft.CodeAnalysis.Diagnostics; + +namespace Confuser.Analyzers { + /// + /// CX001 — flags Import(...) whose argument is a member resolved via reflection on + /// the host runtime (someType.GetMethod(...), GetConstructor, GetField, + /// GetProperty where the receiver is a ). Importing a + /// host-runtime member into a target module produces a reference to the wrong corlib + /// (e.g. the obfuscator's .NET rather than the target's), which breaks the protected + /// assembly. Resolve the member through the target module's corlib instead. + /// + [DiagnosticAnalyzer(LanguageNames.CSharp)] + public sealed class HostRuntimeImportAnalyzer : DiagnosticAnalyzer { + static readonly DiagnosticDescriptor Rule = new DiagnosticDescriptor( + DiagnosticIds.HostRuntimeImport, + "Importing a member resolved via host reflection", + "Import() argument is resolved with Type.{0}() on the host runtime; resolve the member through the target module's corlib to avoid a wrong-corlib reference", + DiagnosticIds.Category, + DiagnosticSeverity.Warning, + isEnabledByDefault: true, + description: "Importing a System.Reflection member (from Type.GetMethod/GetConstructor/GetField/" + + "GetProperty) into a dnlib module references the obfuscator's runtime corlib rather than the " + + "target's, producing a broken assembly. Resolve the member through the target module's corlib."); + + static readonly ImmutableHashSet ReflectionGetters = + ImmutableHashSet.Create("GetMethod", "GetConstructor", "GetField", "GetProperty"); + + public override ImmutableArray SupportedDiagnostics => ImmutableArray.Create(Rule); + + public override void Initialize(AnalysisContext context) { + context.EnableConcurrentExecution(); + context.ConfigureGeneratedCodeAnalysis(GeneratedCodeAnalysisFlags.None); + context.RegisterSyntaxNodeAction(Analyze, SyntaxKind.InvocationExpression); + } + + static void Analyze(SyntaxNodeAnalysisContext context) { + var invocation = (InvocationExpressionSyntax)context.Node; + if (invocation.Expression is not MemberAccessExpressionSyntax member || + member.Name.Identifier.ValueText != "Import") + return; + + foreach (var argument in invocation.ArgumentList.Arguments) { + if (argument.Expression is not InvocationExpressionSyntax inner || + inner.Expression is not MemberAccessExpressionSyntax innerMember) + continue; + + var getterName = innerMember.Name.Identifier.ValueText; + if (!ReflectionGetters.Contains(getterName)) + continue; + + if (context.SemanticModel.GetSymbolInfo(inner, context.CancellationToken).Symbol is not IMethodSymbol getter || + getter.ContainingType?.ToDisplayString() != "System.Type") + continue; + + context.ReportDiagnostic(Diagnostic.Create(Rule, innerMember.Name.GetLocation(), getterName)); + return; + } + } + } +} diff --git a/Confuser.Analyzers/UnguardedResolveAnalyzer.cs b/Confuser.Analyzers/UnguardedResolveAnalyzer.cs new file mode 100644 index 000000000..e825382d7 --- /dev/null +++ b/Confuser.Analyzers/UnguardedResolveAnalyzer.cs @@ -0,0 +1,76 @@ +using System.Collections.Immutable; +using Microsoft.CodeAnalysis; +using Microsoft.CodeAnalysis.CSharp; +using Microsoft.CodeAnalysis.CSharp.Syntax; +using Microsoft.CodeAnalysis.Diagnostics; + +namespace Confuser.Analyzers { + /// + /// CX002 — flags the result of a non-throwing dnlib ResolveTypeDef() / + /// ResolveMethodDef() being dereferenced immediately, with no null check. Those + /// methods return null for external or unresolvable references, so a direct + /// dereference crashes with a on assemblies + /// that reference types outside the obfuscation set. + /// + /// + /// Only a genuine dereference of the result is flagged — x.ResolveTypeDef().Member or + /// x.ResolveMethodDef()[i]. A null-conditional (?.), an assignment, a + /// return, or passing the result as an argument (e.g. to dnlib's null-tolerant + /// SigComparer.Equals) is not a crash and is intentionally not reported. + /// + [DiagnosticAnalyzer(LanguageNames.CSharp)] + public sealed class UnguardedResolveAnalyzer : DiagnosticAnalyzer { + static readonly DiagnosticDescriptor Rule = new DiagnosticDescriptor( + DiagnosticIds.UnguardedResolve, + "Resolve result dereferenced without a null check", + "'{0}()' returns null for unresolvable references; check the result for null before dereferencing it", + DiagnosticIds.Category, + DiagnosticSeverity.Warning, + isEnabledByDefault: true, + description: "dnlib's ResolveTypeDef()/ResolveMethodDef() return null when a reference cannot be " + + "resolved (external or unresolvable types). Dereferencing the result directly crashes obfuscation " + + "on such assemblies; guard it with a null check or the null-conditional operator."); + + static readonly ImmutableHashSet ResolveMethods = + ImmutableHashSet.Create("ResolveTypeDef", "ResolveMethodDef"); + + public override ImmutableArray SupportedDiagnostics => ImmutableArray.Create(Rule); + + public override void Initialize(AnalysisContext context) { + context.EnableConcurrentExecution(); + context.ConfigureGeneratedCodeAnalysis(GeneratedCodeAnalysisFlags.None); + context.RegisterSyntaxNodeAction(Analyze, SyntaxKind.InvocationExpression); + } + + static void Analyze(SyntaxNodeAnalysisContext context) { + var invocation = (InvocationExpressionSyntax)context.Node; + if (invocation.Expression is not MemberAccessExpressionSyntax member || + !ResolveMethods.Contains(member.Name.Identifier.ValueText) || + invocation.ArgumentList.Arguments.Count != 0) + return; + + if (!IsImmediatelyDereferenced(invocation)) + return; + + context.ReportDiagnostic(Diagnostic.Create(Rule, member.Name.GetLocation(), + member.Name.Identifier.ValueText)); + } + + /// + /// Returns true only when is the receiver of a + /// plain member access or element access — i.e. its result is dereferenced right away + /// without a null guard. A null-conditional access makes the parent a + /// , which is safe and returns false. + /// + static bool IsImmediatelyDereferenced(InvocationExpressionSyntax invocation) { + switch (invocation.Parent) { + case MemberAccessExpressionSyntax parentMember when parentMember.Expression == invocation: + return true; + case ElementAccessExpressionSyntax parentElement when parentElement.Expression == invocation: + return true; + default: + return false; + } + } + } +} diff --git a/Confuser.Protections/Utils.cs b/Confuser.Protections/Utils.cs index e46f701c1..aa7c79494 100644 --- a/Confuser.Protections/Utils.cs +++ b/Confuser.Protections/Utils.cs @@ -12,7 +12,15 @@ internal static class Utils { public static IMethod Import(this ModuleDef module, ConfuserContext context, Type classType, string method) { var corLib = context.Resolver.Resolve(context.CurrentModule?.CorLibTypes.AssemblyRef, context.CurrentModule); var typeInfo = corLib?.ManifestModule.Find(classType.FullName, true); - return (typeInfo == null) ? module.Import(classType.GetMethod(method)) : module.Import(typeInfo.FindMethod(method)); + if (typeInfo != null) + return module.Import(typeInfo.FindMethod(method)); + + // CX001: intentional last-resort fallback. When the target module's corlib type cannot be + // resolved through the context (rare), host reflection is the only available source for the + // member reference. New code must resolve through the context instead of copying this. +#pragma warning disable CX001 + return module.Import(classType.GetMethod(method)); +#pragma warning restore CX001 } } } diff --git a/Tests/Confuser.Analyzers.Test/HostRuntimeImportAnalyzerTest.cs b/Tests/Confuser.Analyzers.Test/HostRuntimeImportAnalyzerTest.cs new file mode 100644 index 000000000..f7c3341b2 --- /dev/null +++ b/Tests/Confuser.Analyzers.Test/HostRuntimeImportAnalyzerTest.cs @@ -0,0 +1,63 @@ +using System.Threading.Tasks; +using Microsoft.CodeAnalysis.Testing; +using Xunit; +using Verify = Microsoft.CodeAnalysis.CSharp.Testing.CSharpAnalyzerVerifier< + Confuser.Analyzers.HostRuntimeImportAnalyzer, + Microsoft.CodeAnalysis.Testing.DefaultVerifier>; + +namespace Confuser.Analyzers.Test { + public class HostRuntimeImportAnalyzerTest { + const string Stub = @" +using System; +class Mod { public object Import(object member) => member; } +"; + + [Fact] + public async Task Flags_ImportOfTypeVariableGetMethod() { + string source = Stub + @" +class C { + void M(Mod mod, Type t) { + mod.Import(t.{|#0:GetMethod|}(""X"")); + } +}"; + var expected = Verify.Diagnostic("CX001").WithLocation(0).WithArguments("GetMethod"); + await Verify.VerifyAnalyzerAsync(source, expected); + } + + [Fact] + public async Task Flags_ImportOfTypeofGetConstructor() { + string source = Stub + @" +class C { + void M(Mod mod) { + mod.Import(typeof(string).{|#0:GetConstructor|}(Type.EmptyTypes)); + } +}"; + var expected = Verify.Diagnostic("CX001").WithLocation(0).WithArguments("GetConstructor"); + await Verify.VerifyAnalyzerAsync(source, expected); + } + + [Fact] + public async Task DoesNotFlag_ImportOfNonReflectionMember() { + // Resolving through a non-System.Type API (the safe path) must not be flagged. + string source = Stub + @" +class Resolved { public object FindMethod(string n) => null; } +class C { + void M(Mod mod, Resolved r) { + mod.Import(r.FindMethod(""X"")); + } +}"; + await Verify.VerifyAnalyzerAsync(source); + } + + [Fact] + public async Task DoesNotFlag_NonImportCall() { + string source = Stub + @" +class C { + void M(Type t) { + var m = t.GetMethod(""X""); + } +}"; + await Verify.VerifyAnalyzerAsync(source); + } + } +} diff --git a/Tests/Confuser.Analyzers.Test/UnguardedResolveAnalyzerTest.cs b/Tests/Confuser.Analyzers.Test/UnguardedResolveAnalyzerTest.cs new file mode 100644 index 000000000..f0cb3eec2 --- /dev/null +++ b/Tests/Confuser.Analyzers.Test/UnguardedResolveAnalyzerTest.cs @@ -0,0 +1,67 @@ +using System.Threading.Tasks; +using Microsoft.CodeAnalysis.Testing; +using Xunit; +using Verify = Microsoft.CodeAnalysis.CSharp.Testing.CSharpAnalyzerVerifier< + Confuser.Analyzers.UnguardedResolveAnalyzer, + Microsoft.CodeAnalysis.Testing.DefaultVerifier>; + +namespace Confuser.Analyzers.Test { + public class UnguardedResolveAnalyzerTest { + const string Stub = @" +class Def { public int Field; public void Do() {} } +class Ref { + public Def ResolveTypeDef() => null; + public Def ResolveMethodDef() => null; +} +"; + + [Fact] + public async Task Flags_ImmediateMemberDereference() { + string source = Stub + @" +class C { + void M(Ref r) { + r.{|#0:ResolveTypeDef|}().Do(); + } +}"; + var expected = Verify.Diagnostic("CX002").WithLocation(0).WithArguments("ResolveTypeDef"); + await Verify.VerifyAnalyzerAsync(source, expected); + } + + [Fact] + public async Task DoesNotFlag_NullConditionalDereference() { + string source = Stub + @" +class C { + void M(Ref r) { + r.ResolveMethodDef()?.Do(); + } +}"; + await Verify.VerifyAnalyzerAsync(source); + } + + [Fact] + public async Task DoesNotFlag_AssignmentWithoutDereference() { + string source = Stub + @" +class C { + Def M(Ref r) { + var d = r.ResolveTypeDef(); + return d; + } +}"; + await Verify.VerifyAnalyzerAsync(source); + } + + [Fact] + public async Task DoesNotFlag_PassedAsArgument() { + // Mirrors the real VTableAnalyzer usage: the result is handed to a null-tolerant method, + // which is not a dereference and must not be flagged. + string source = Stub + @" +class C { + static bool AreEqual(Def a, Def b) => true; + void M(Ref r) { + var ok = AreEqual(r.ResolveTypeDef(), r.ResolveMethodDef()); + } +}"; + await Verify.VerifyAnalyzerAsync(source); + } + } +}