Skip to content

Commit 8afdbfa

Browse files
author
RandomCrocodile
committed
feature: add CX002 and CX001 analyzers, completing Tier 3 (#49)
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.
1 parent d1cf356 commit 8afdbfa

5 files changed

Lines changed: 279 additions & 1 deletion

File tree

Lines changed: 64 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,64 @@
1+
using System.Collections.Immutable;
2+
using Microsoft.CodeAnalysis;
3+
using Microsoft.CodeAnalysis.CSharp;
4+
using Microsoft.CodeAnalysis.CSharp.Syntax;
5+
using Microsoft.CodeAnalysis.Diagnostics;
6+
7+
namespace Confuser.Analyzers {
8+
/// <summary>
9+
/// CX001 — flags <c>Import(...)</c> whose argument is a member resolved via reflection on
10+
/// the host runtime (<c>someType.GetMethod(...)</c>, <c>GetConstructor</c>, <c>GetField</c>,
11+
/// <c>GetProperty</c> where the receiver is a <see cref="System.Type" />). Importing a
12+
/// host-runtime member into a target module produces a reference to the wrong corlib
13+
/// (e.g. the obfuscator's .NET rather than the target's), which breaks the protected
14+
/// assembly. Resolve the member through the target module's corlib instead.
15+
/// </summary>
16+
[DiagnosticAnalyzer(LanguageNames.CSharp)]
17+
public sealed class HostRuntimeImportAnalyzer : DiagnosticAnalyzer {
18+
static readonly DiagnosticDescriptor Rule = new DiagnosticDescriptor(
19+
DiagnosticIds.HostRuntimeImport,
20+
"Importing a member resolved via host reflection",
21+
"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",
22+
DiagnosticIds.Category,
23+
DiagnosticSeverity.Warning,
24+
isEnabledByDefault: true,
25+
description: "Importing a System.Reflection member (from Type.GetMethod/GetConstructor/GetField/" +
26+
"GetProperty) into a dnlib module references the obfuscator's runtime corlib rather than the " +
27+
"target's, producing a broken assembly. Resolve the member through the target module's corlib.");
28+
29+
static readonly ImmutableHashSet<string> ReflectionGetters =
30+
ImmutableHashSet.Create("GetMethod", "GetConstructor", "GetField", "GetProperty");
31+
32+
public override ImmutableArray<DiagnosticDescriptor> SupportedDiagnostics => ImmutableArray.Create(Rule);
33+
34+
public override void Initialize(AnalysisContext context) {
35+
context.EnableConcurrentExecution();
36+
context.ConfigureGeneratedCodeAnalysis(GeneratedCodeAnalysisFlags.None);
37+
context.RegisterSyntaxNodeAction(Analyze, SyntaxKind.InvocationExpression);
38+
}
39+
40+
static void Analyze(SyntaxNodeAnalysisContext context) {
41+
var invocation = (InvocationExpressionSyntax)context.Node;
42+
if (invocation.Expression is not MemberAccessExpressionSyntax member ||
43+
member.Name.Identifier.ValueText != "Import")
44+
return;
45+
46+
foreach (var argument in invocation.ArgumentList.Arguments) {
47+
if (argument.Expression is not InvocationExpressionSyntax inner ||
48+
inner.Expression is not MemberAccessExpressionSyntax innerMember)
49+
continue;
50+
51+
var getterName = innerMember.Name.Identifier.ValueText;
52+
if (!ReflectionGetters.Contains(getterName))
53+
continue;
54+
55+
if (context.SemanticModel.GetSymbolInfo(inner, context.CancellationToken).Symbol is not IMethodSymbol getter ||
56+
getter.ContainingType?.ToDisplayString() != "System.Type")
57+
continue;
58+
59+
context.ReportDiagnostic(Diagnostic.Create(Rule, innerMember.Name.GetLocation(), getterName));
60+
return;
61+
}
62+
}
63+
}
64+
}
Lines changed: 76 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,76 @@
1+
using System.Collections.Immutable;
2+
using Microsoft.CodeAnalysis;
3+
using Microsoft.CodeAnalysis.CSharp;
4+
using Microsoft.CodeAnalysis.CSharp.Syntax;
5+
using Microsoft.CodeAnalysis.Diagnostics;
6+
7+
namespace Confuser.Analyzers {
8+
/// <summary>
9+
/// CX002 — flags the result of a non-throwing dnlib <c>ResolveTypeDef()</c> /
10+
/// <c>ResolveMethodDef()</c> being dereferenced immediately, with no null check. Those
11+
/// methods return <c>null</c> for external or unresolvable references, so a direct
12+
/// dereference crashes with a <see cref="System.NullReferenceException" /> on assemblies
13+
/// that reference types outside the obfuscation set.
14+
/// </summary>
15+
/// <remarks>
16+
/// Only a genuine dereference of the result is flagged — <c>x.ResolveTypeDef().Member</c> or
17+
/// <c>x.ResolveMethodDef()[i]</c>. A null-conditional (<c>?.</c>), an assignment, a
18+
/// <c>return</c>, or passing the result as an argument (e.g. to dnlib's null-tolerant
19+
/// <c>SigComparer.Equals</c>) is not a crash and is intentionally not reported.
20+
/// </remarks>
21+
[DiagnosticAnalyzer(LanguageNames.CSharp)]
22+
public sealed class UnguardedResolveAnalyzer : DiagnosticAnalyzer {
23+
static readonly DiagnosticDescriptor Rule = new DiagnosticDescriptor(
24+
DiagnosticIds.UnguardedResolve,
25+
"Resolve result dereferenced without a null check",
26+
"'{0}()' returns null for unresolvable references; check the result for null before dereferencing it",
27+
DiagnosticIds.Category,
28+
DiagnosticSeverity.Warning,
29+
isEnabledByDefault: true,
30+
description: "dnlib's ResolveTypeDef()/ResolveMethodDef() return null when a reference cannot be " +
31+
"resolved (external or unresolvable types). Dereferencing the result directly crashes obfuscation " +
32+
"on such assemblies; guard it with a null check or the null-conditional operator.");
33+
34+
static readonly ImmutableHashSet<string> ResolveMethods =
35+
ImmutableHashSet.Create("ResolveTypeDef", "ResolveMethodDef");
36+
37+
public override ImmutableArray<DiagnosticDescriptor> SupportedDiagnostics => ImmutableArray.Create(Rule);
38+
39+
public override void Initialize(AnalysisContext context) {
40+
context.EnableConcurrentExecution();
41+
context.ConfigureGeneratedCodeAnalysis(GeneratedCodeAnalysisFlags.None);
42+
context.RegisterSyntaxNodeAction(Analyze, SyntaxKind.InvocationExpression);
43+
}
44+
45+
static void Analyze(SyntaxNodeAnalysisContext context) {
46+
var invocation = (InvocationExpressionSyntax)context.Node;
47+
if (invocation.Expression is not MemberAccessExpressionSyntax member ||
48+
!ResolveMethods.Contains(member.Name.Identifier.ValueText) ||
49+
invocation.ArgumentList.Arguments.Count != 0)
50+
return;
51+
52+
if (!IsImmediatelyDereferenced(invocation))
53+
return;
54+
55+
context.ReportDiagnostic(Diagnostic.Create(Rule, member.Name.GetLocation(),
56+
member.Name.Identifier.ValueText));
57+
}
58+
59+
/// <summary>
60+
/// Returns <c>true</c> only when <paramref name="invocation" /> is the receiver of a
61+
/// plain member access or element access — i.e. its result is dereferenced right away
62+
/// without a null guard. A null-conditional access makes the parent a
63+
/// <see cref="ConditionalAccessExpressionSyntax" />, which is safe and returns false.
64+
/// </summary>
65+
static bool IsImmediatelyDereferenced(InvocationExpressionSyntax invocation) {
66+
switch (invocation.Parent) {
67+
case MemberAccessExpressionSyntax parentMember when parentMember.Expression == invocation:
68+
return true;
69+
case ElementAccessExpressionSyntax parentElement when parentElement.Expression == invocation:
70+
return true;
71+
default:
72+
return false;
73+
}
74+
}
75+
}
76+
}

Confuser.Protections/Utils.cs

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,15 @@ internal static class Utils {
1212
public static IMethod Import(this ModuleDef module, ConfuserContext context, Type classType, string method) {
1313
var corLib = context.Resolver.Resolve(context.CurrentModule?.CorLibTypes.AssemblyRef, context.CurrentModule);
1414
var typeInfo = corLib?.ManifestModule.Find(classType.FullName, true);
15-
return (typeInfo == null) ? module.Import(classType.GetMethod(method)) : module.Import(typeInfo.FindMethod(method));
15+
if (typeInfo != null)
16+
return module.Import(typeInfo.FindMethod(method));
17+
18+
// CX001: intentional last-resort fallback. When the target module's corlib type cannot be
19+
// resolved through the context (rare), host reflection is the only available source for the
20+
// member reference. New code must resolve through the context instead of copying this.
21+
#pragma warning disable CX001
22+
return module.Import(classType.GetMethod(method));
23+
#pragma warning restore CX001
1624
}
1725
}
1826
}
Lines changed: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,63 @@
1+
using System.Threading.Tasks;
2+
using Microsoft.CodeAnalysis.Testing;
3+
using Xunit;
4+
using Verify = Microsoft.CodeAnalysis.CSharp.Testing.CSharpAnalyzerVerifier<
5+
Confuser.Analyzers.HostRuntimeImportAnalyzer,
6+
Microsoft.CodeAnalysis.Testing.DefaultVerifier>;
7+
8+
namespace Confuser.Analyzers.Test {
9+
public class HostRuntimeImportAnalyzerTest {
10+
const string Stub = @"
11+
using System;
12+
class Mod { public object Import(object member) => member; }
13+
";
14+
15+
[Fact]
16+
public async Task Flags_ImportOfTypeVariableGetMethod() {
17+
string source = Stub + @"
18+
class C {
19+
void M(Mod mod, Type t) {
20+
mod.Import(t.{|#0:GetMethod|}(""X""));
21+
}
22+
}";
23+
var expected = Verify.Diagnostic("CX001").WithLocation(0).WithArguments("GetMethod");
24+
await Verify.VerifyAnalyzerAsync(source, expected);
25+
}
26+
27+
[Fact]
28+
public async Task Flags_ImportOfTypeofGetConstructor() {
29+
string source = Stub + @"
30+
class C {
31+
void M(Mod mod) {
32+
mod.Import(typeof(string).{|#0:GetConstructor|}(Type.EmptyTypes));
33+
}
34+
}";
35+
var expected = Verify.Diagnostic("CX001").WithLocation(0).WithArguments("GetConstructor");
36+
await Verify.VerifyAnalyzerAsync(source, expected);
37+
}
38+
39+
[Fact]
40+
public async Task DoesNotFlag_ImportOfNonReflectionMember() {
41+
// Resolving through a non-System.Type API (the safe path) must not be flagged.
42+
string source = Stub + @"
43+
class Resolved { public object FindMethod(string n) => null; }
44+
class C {
45+
void M(Mod mod, Resolved r) {
46+
mod.Import(r.FindMethod(""X""));
47+
}
48+
}";
49+
await Verify.VerifyAnalyzerAsync(source);
50+
}
51+
52+
[Fact]
53+
public async Task DoesNotFlag_NonImportCall() {
54+
string source = Stub + @"
55+
class C {
56+
void M(Type t) {
57+
var m = t.GetMethod(""X"");
58+
}
59+
}";
60+
await Verify.VerifyAnalyzerAsync(source);
61+
}
62+
}
63+
}
Lines changed: 67 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,67 @@
1+
using System.Threading.Tasks;
2+
using Microsoft.CodeAnalysis.Testing;
3+
using Xunit;
4+
using Verify = Microsoft.CodeAnalysis.CSharp.Testing.CSharpAnalyzerVerifier<
5+
Confuser.Analyzers.UnguardedResolveAnalyzer,
6+
Microsoft.CodeAnalysis.Testing.DefaultVerifier>;
7+
8+
namespace Confuser.Analyzers.Test {
9+
public class UnguardedResolveAnalyzerTest {
10+
const string Stub = @"
11+
class Def { public int Field; public void Do() {} }
12+
class Ref {
13+
public Def ResolveTypeDef() => null;
14+
public Def ResolveMethodDef() => null;
15+
}
16+
";
17+
18+
[Fact]
19+
public async Task Flags_ImmediateMemberDereference() {
20+
string source = Stub + @"
21+
class C {
22+
void M(Ref r) {
23+
r.{|#0:ResolveTypeDef|}().Do();
24+
}
25+
}";
26+
var expected = Verify.Diagnostic("CX002").WithLocation(0).WithArguments("ResolveTypeDef");
27+
await Verify.VerifyAnalyzerAsync(source, expected);
28+
}
29+
30+
[Fact]
31+
public async Task DoesNotFlag_NullConditionalDereference() {
32+
string source = Stub + @"
33+
class C {
34+
void M(Ref r) {
35+
r.ResolveMethodDef()?.Do();
36+
}
37+
}";
38+
await Verify.VerifyAnalyzerAsync(source);
39+
}
40+
41+
[Fact]
42+
public async Task DoesNotFlag_AssignmentWithoutDereference() {
43+
string source = Stub + @"
44+
class C {
45+
Def M(Ref r) {
46+
var d = r.ResolveTypeDef();
47+
return d;
48+
}
49+
}";
50+
await Verify.VerifyAnalyzerAsync(source);
51+
}
52+
53+
[Fact]
54+
public async Task DoesNotFlag_PassedAsArgument() {
55+
// Mirrors the real VTableAnalyzer usage: the result is handed to a null-tolerant method,
56+
// which is not a dereference and must not be flagged.
57+
string source = Stub + @"
58+
class C {
59+
static bool AreEqual(Def a, Def b) => true;
60+
void M(Ref r) {
61+
var ok = AreEqual(r.ResolveTypeDef(), r.ResolveMethodDef());
62+
}
63+
}";
64+
await Verify.VerifyAnalyzerAsync(source);
65+
}
66+
}
67+
}

0 commit comments

Comments
 (0)