Skip to content

Commit 996788d

Browse files
committed
Improve nested type and state machine handling in IL extraction
Normalized nested type names from '/' to '.' for consistency in ILExtractor and method call targets. Added extraction of nested types, including compiler-generated state machines. Updated VulnerableCalls.qll to link iterator/async stub methods to their state machine implementations, improving vulnerability tracking. Updated codeql-extractor.yml to specify dbscheme.
1 parent 668950a commit 996788d

3 files changed

Lines changed: 69 additions & 9 deletions

File tree

binary/extractor/cil/Semmle.Extraction.CSharp.IL/ILExtractor.cs

Lines changed: 25 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -32,10 +32,6 @@ public void Extract(string dllPath) {
3232

3333
foreach (var module in assembly.Modules) {
3434
foreach (var type in module.Types) {
35-
// Skip compiler-generated types for now
36-
if (type.Name.Contains("<") || type.Name.StartsWith("<"))
37-
continue;
38-
3935
ExtractType(type);
4036
}
4137
}
@@ -64,8 +60,22 @@ private void ExtractType(TypeDefinition type) {
6460
var typeId = trap.GetId();
6561
typeIds[type.FullName] = typeId;
6662

63+
// For nested types, we need to construct proper namespace and name
64+
// Cecil uses '/' for nested types (e.g., "Outer/Inner") but the call targets use '.'
65+
// So we normalize to dot notation for consistency
66+
var fullName = type.FullName.Replace('/', '.');
67+
var typeName = type.Name;
68+
string typeNamespace;
69+
70+
if (type.IsNested && type.DeclaringType != null) {
71+
// For nested types, the namespace is the parent type's full name
72+
typeNamespace = type.DeclaringType.FullName.Replace('/', '.');
73+
} else {
74+
typeNamespace = type.Namespace;
75+
}
76+
6777
// Write type info
68-
trap.WriteTuple("types", typeId, type.FullName, type.Namespace, type.Name);
78+
trap.WriteTuple("types", typeId, fullName, typeNamespace, typeName);
6979

7080
foreach (var method in type.Methods) {
7181
// Skip some special methods
@@ -74,6 +84,13 @@ private void ExtractType(TypeDefinition type) {
7484

7585
ExtractMethod(method, typeId);
7686
}
87+
88+
// Extract nested types (includes compiler-generated state machines)
89+
if (type.HasNestedTypes) {
90+
foreach (var nestedType in type.NestedTypes) {
91+
ExtractType(nestedType);
92+
}
93+
}
7794
}
7895

7996
private void ExtractMethod(MethodDefinition method, int typeId) {
@@ -146,9 +163,9 @@ private void ExtractMethodBody(MethodDefinition method, int methodId) {
146163
// Branch target
147164
trap.WriteTuple("il_branch_target", instrId, targetInstr.Offset);
148165
} else if (instruction.Operand is MethodReference methodRef) {
149-
// Method call - we'll resolve this in a second pass
150-
var targetMethodName =
151-
$"{methodRef.DeclaringType.FullName}.{methodRef.Name}";
166+
// Method call - normalize nested type separators from '/' to '.'
167+
var declaringTypeName = methodRef.DeclaringType.FullName.Replace('/', '.');
168+
var targetMethodName = $"{declaringTypeName}.{methodRef.Name}";
152169
trap.WriteTuple("il_call_target_unresolved", instrId, targetMethodName);
153170
trap.WriteTuple("il_number_of_arguments", instrId, methodRef.Parameters.Count);
154171
if(methodRef.MethodReturnType.ReturnType.MetadataType is not Mono.Cecil.MetadataType.Void) {

binary/extractor/cil/codeql-extractor.yml

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,4 +12,5 @@ file_types:
1212
display_name: C# IL
1313
extensions:
1414
- .exe
15-
- .dll
15+
- .dll
16+
dbscheme: semmlecode.binary.dbscheme

binary/ql/src/VulnerableCalls/VulnerableCalls.qll

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -48,9 +48,44 @@ Function getADirectlyVulnerableMethod(string id) {
4848
result = getAVulnerableCallFromModel(id).getEnclosingFunction()
4949
}
5050

51+
/**
52+
* Holds if `stub` is an iterator/async stub method and `stateMachine` is its
53+
* corresponding state machine implementation (the MoveNext method).
54+
*
55+
* Iterator/async methods in C# are compiled to:
56+
* 1. A stub method that creates a state machine object
57+
* 2. A nested class with a MoveNext method containing the actual implementation
58+
*
59+
* The pattern is: method `Foo` in class `Bar` creates `Bar.<Foo>d__N`
60+
* and the impl is in `Bar.<Foo>d__N.MoveNext`
61+
*/
62+
private predicate isStateMachineImplementation(Function stub, Function stateMachine) {
63+
exists(string stubName, Type stubType, string stateMachineTypeName |
64+
stubName = stub.getName() and
65+
stubType = stub.getDeclaringType() and
66+
stateMachine.getName() = "MoveNext" and
67+
// The state machine type is nested in the same type as the stub
68+
// and named <MethodName>d__N
69+
stateMachineTypeName = stateMachine.getDeclaringType().getName() and
70+
stateMachineTypeName.matches("<" + stubName + ">d__%" ) and
71+
// The state machine's declaring type's namespace should be the stub's type full name
72+
stateMachine.getDeclaringType().getNamespace() = stubType.getFullName()
73+
)
74+
}
75+
76+
/**
77+
* Gets the state machine implementation for an iterator/async stub method.
78+
*/
79+
Function getStateMachineImplementation(Function stub) {
80+
isStateMachineImplementation(stub, result)
81+
}
82+
5183
/**
5284
* Gets a method that transitively calls a vulnerable method.
5385
* This computes the transitive closure of the call graph.
86+
*
87+
* Also handles iterator/async methods by linking stub methods to their
88+
* state machine implementations.
5489
*/
5590
Function getAVulnerableMethod(string id) {
5691
// Direct call to vulnerable method
@@ -63,6 +98,13 @@ Function getAVulnerableMethod(string id) {
6398
call.getTargetOperand().getAnyDef().(ExternalRefInstruction).getFullyQualifiedName() =
6499
callee.getFullyQualifiedName()
65100
)
101+
or
102+
// Iterator/async: if a state machine's MoveNext is vulnerable,
103+
// the stub method that creates it is also vulnerable
104+
exists(Function stateMachine |
105+
stateMachine = getAVulnerableMethod(id) and
106+
isStateMachineImplementation(result, stateMachine)
107+
)
66108
}
67109

68110
/**

0 commit comments

Comments
 (0)