feature: Tier 3 custom Roslyn analyzers CX001–CX004 (#49, #46)

Confuser.Analyzers/Confuser.Analyzers.csproj

@@ -0,0 +1,26 @@
+<Project Sdk="Microsoft.NET.Sdk">
+
+  <!-- Roslyn analyzer assembly. Deliberately standalone (does NOT import
+       ConfuserEx.Common.props) — analyzers must be a single netstandard2.0 assembly
+       with no runtime output and no strong-name/multi-target concerns. -->
+  <PropertyGroup>
+    <TargetFramework>netstandard2.0</TargetFramework>
+    <IncludeBuildOutput>false</IncludeBuildOutput>
+    <Nullable>enable</Nullable>
+    <LangVersion>latest</LangVersion>
+    <EnforceExtendedAnalyzerRules>true</EnforceExtendedAnalyzerRules>
+    <IsPackable>false</IsPackable>
+    <!-- These analyzers ship inside this repo only; they are never consumed as a NuGet package. -->
+    <GeneratePackageOnBuild>false</GeneratePackageOnBuild>
+    <!-- RS2008 (analyzer release tracking) only matters for analyzers shipped as packages. -->
+    <NoWarn>$(NoWarn);RS2008</NoWarn>
+  </PropertyGroup>
+
+  <ItemGroup>
+    <PackageReference Include="Microsoft.CodeAnalysis.CSharp" Version="4.8.0" PrivateAssets="all" />
+    <PackageReference Include="Microsoft.CodeAnalysis.Analyzers" Version="3.3.4" PrivateAssets="all">
+      <IncludeAssets>runtime; build; native; contentfiles; analyzers; buildtransitive</IncludeAssets>
+    </PackageReference>
+  </ItemGroup>
+
+</Project>

Confuser.Analyzers/DiagnosticIds.cs

@@ -0,0 +1,21 @@
+namespace Confuser.Analyzers {
+	/// <summary>
+	///     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.
+	/// </summary>
+	internal static class DiagnosticIds {
+		/// <summary>Imports a member from the host runtime instead of the target's corlib.</summary>
+		public const string HostRuntimeImport = "CX001";
+
+		/// <summary><c>ResolveTypeDef()</c>/<c>ResolveMethodDef()</c> used without a null check.</summary>
+		public const string UnguardedResolve = "CX002";
+
+		/// <summary>Usage of a <c>...Throw</c> resolve helper (audit / awareness).</summary>
+		public const string ResolveThrowAudit = "CX003";
+
+		/// <summary><c>GetTypes()</c> without handling <c>ReflectionTypeLoadException</c>.</summary>
+		public const string UnhandledReflectionTypeLoad = "CX004";
+
+		public const string Category = "ConfuserEx";
+	}
+}

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 {
+	/// <summary>
+	///     CX001 — flags <c>Import(...)</c> whose argument is a member resolved via reflection on
+	///     the host runtime (<c>someType.GetMethod(...)</c>, <c>GetConstructor</c>, <c>GetField</c>,
+	///     <c>GetProperty</c> where the receiver is a <see cref="System.Type" />). 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.
+	/// </summary>
+	[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<string> ReflectionGetters =
+			ImmutableHashSet.Create("GetMethod", "GetConstructor", "GetField", "GetProperty");
+
+		public override ImmutableArray<DiagnosticDescriptor> 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;
+			}
+		}
+	}
+}

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 {
+	/// <summary>
+	///     CX003 — an awareness/audit rule that surfaces every call to a dnlib <c>Resolve…Throw</c>
+	///     helper (<c>ResolveThrow</c>, <c>ResolveTypeDefThrow</c>, <c>ResolveMethodDefThrow</c>,
+	///     <c>ResolveFieldThrow</c>). These throw when a reference cannot be resolved, which crashes
+	///     obfuscation on assemblies with external/unresolvable members. Most uses are intentional;
+	///     this reports at <see cref="DiagnosticSeverity.Info" /> so the spots can be evaluated
+	///     without adding build noise.
+	/// </summary>
+	[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<DiagnosticDescriptor> 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);
+	}
+}

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 {
+	/// <summary>
+	///     CX002 — flags the result of a non-throwing dnlib <c>ResolveTypeDef()</c> /
+	///     <c>ResolveMethodDef()</c> being dereferenced immediately, with no null check. Those
+	///     methods return <c>null</c> for external or unresolvable references, so a direct
+	///     dereference crashes with a <see cref="System.NullReferenceException" /> on assemblies
+	///     that reference types outside the obfuscation set.
+	/// </summary>
+	/// <remarks>
+	///     Only a genuine dereference of the result is flagged — <c>x.ResolveTypeDef().Member</c> or
+	///     <c>x.ResolveMethodDef()[i]</c>. A null-conditional (<c>?.</c>), an assignment, a
+	///     <c>return</c>, or passing the result as an argument (e.g. to dnlib's null-tolerant
+	///     <c>SigComparer.Equals</c>) is not a crash and is intentionally not reported.
+	/// </remarks>
+	[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<string> ResolveMethods =
+			ImmutableHashSet.Create("ResolveTypeDef", "ResolveMethodDef");
+
+		public override ImmutableArray<DiagnosticDescriptor> 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));
+		}
+
+		/// <summary>
+		///     Returns <c>true</c> only when <paramref name="invocation" /> 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
+		///     <see cref="ConditionalAccessExpressionSyntax" />, which is safe and returns false.
+		/// </summary>
+		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;
+			}
+		}
+	}
+}

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 {
+	/// <summary>
+	///     CX004 — flags <c>Assembly.GetTypes()</c> / <c>Module.GetTypes()</c> that are not guarded
+	///     against <see cref="System.Reflection.ReflectionTypeLoadException" />. That exception is
+	///     thrown whenever a contained type cannot be loaded (common for plugin assemblies with
+	///     unresolved dependencies) and caused packer/plugin startup crashes.
+	/// </summary>
+	[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<DiagnosticDescriptor> 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;
+		}
+	}
+}

Confuser.Core/PluginDiscovery.cs

@@ -55,8 +55,18 @@ public static bool HasAccessibleDefConstructor(Type type) {
 		protected static void AddPlugins(
 			ConfuserContext context, IList<Protection> protections, IList<Packer> packers,
 			IList<ConfuserComponent> 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();
 		}
 

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
 		}
 	}
 }