Skip to content

Commit 5a20e85

Browse files
geoffw0geoffw0
authored
Merge pull request #2638 from jbj/ir-dispatch
C++ IR: Support for global virtual dispatch
2 parents 742bd1c + 391b80e commit 5a20e85

File tree

4 files changed

+221
-46
lines changed

4 files changed

+221
-46
lines changed

‎cpp/ql/src/semmle/code/cpp/ir/dataflow/internal/DataFlowDispatch.qll‎

Lines changed: 123 additions & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
private import cpp
22
private import semmle.code.cpp.ir.IR
33
private import semmle.code.cpp.ir.dataflow.DataFlow
4+
private import semmle.code.cpp.ir.dataflow.internal.DataFlowPrivate
45

56
Function viableImpl(CallInstruction call) { result = viableCallable(call) }
67

@@ -22,57 +23,133 @@ Function viableCallable(CallInstruction call) {
2223
strictcount(Function other | functionSignatureWithBody(qualifiedName, nparams, other)) = 1
2324
)
2425
or
25-
// Rudimentary virtual dispatch support. It's essentially local data flow
26-
// where the source is a derived-to-base conversion and the target is the
27-
// qualifier of a call.
28-
exists(Class derived, DataFlow::Node thisArgument |
29-
nodeMayHaveClass(derived, thisArgument) and
30-
overrideMayAffectCall(derived, thisArgument, _, result, call)
31-
)
26+
// Virtual dispatch
27+
result = call.(VirtualDispatch::DataSensitiveCall).resolve()
3228
}
3329

3430
/**
35-
* Holds if `call` is a virtual function call with qualifier `thisArgument` in
36-
* `enclosingFunction`, whose static target is overridden by
37-
* `overridingFunction` in `overridingClass`.
31+
* Provides virtual dispatch support compatible with the original
32+
* implementation of `semmle.code.cpp.security.TaintTracking`.
3833
*/
39-
pragma[noinline]
40-
private predicate overrideMayAffectCall(
41-
Class overridingClass, DataFlow::Node thisArgument, Function enclosingFunction,
42-
MemberFunction overridingFunction, CallInstruction call
43-
) {
44-
call.getEnclosingFunction() = enclosingFunction and
45-
overridingFunction.getAnOverriddenFunction+() = call.getStaticCallTarget() and
46-
overridingFunction.getDeclaringType() = overridingClass and
47-
thisArgument = DataFlow::instructionNode(call.getThisArgument())
48-
}
34+
private module VirtualDispatch {
35+
/** A call that may dispatch differently depending on the qualifier value. */
36+
abstract class DataSensitiveCall extends DataFlowCall {
37+
/**
38+
* Gets the node whose value determines the target of this call. This node
39+
* could be the qualifier of a virtual dispatch or the function-pointer
40+
* expression in a call to a function pointer. What they have in common is
41+
* that we need to find out which data flows there, and then it's up to the
42+
* `resolve` predicate to stitch that information together and resolve the
43+
* call.
44+
*/
45+
abstract DataFlow::Node getDispatchValue();
4946

50-
/**
51-
* Holds if `node` may have dynamic class `derived`, where `derived` is a class
52-
* that may affect virtual dispatch within the enclosing function.
53-
*
54-
* For the sake of performance, this recursion is written out manually to make
55-
* it a relation on `Class x Node` rather than `Node x Node` or `MemberFunction
56-
* x Node`, both of which would be larger. It's a forward search since there
57-
* should usually be fewer classes than calls.
58-
*
59-
* If a value is cast several classes up in the hierarchy, that will be modeled
60-
* as a chain of `ConvertToBaseInstruction`s and will cause the search to start
61-
* from each of them and pass through subsequent ones. There might be
62-
* performance to gain by stopping before a second upcast and reconstructing
63-
* the full chain in a "big-step" recursion after this one.
64-
*/
65-
private predicate nodeMayHaveClass(Class derived, DataFlow::Node node) {
66-
exists(ConvertToBaseInstruction toBase |
67-
derived = toBase.getDerivedClass() and
68-
overrideMayAffectCall(derived, _, toBase.getEnclosingFunction(), _, _) and
69-
node.asInstruction() = toBase
70-
)
71-
or
72-
exists(DataFlow::Node prev |
73-
nodeMayHaveClass(derived, prev) and
74-
DataFlow::localFlowStep(prev, node)
75-
)
47+
/** Gets a candidate target for this call. */
48+
cached
49+
abstract Function resolve();
50+
51+
/**
52+
* Whether `src` can flow to this call.
53+
*
54+
* Searches backwards from `getDispatchValue()` to `src`. The `allowFromArg`
55+
* parameter is true when the search is allowed to continue backwards into
56+
* a parameter; non-recursive callers should pass `_` for `allowFromArg`.
57+
*/
58+
predicate flowsFrom(DataFlow::Node src, boolean allowFromArg) {
59+
src = this.getDispatchValue() and allowFromArg = true
60+
or
61+
exists(DataFlow::Node other, boolean allowOtherFromArg |
62+
this.flowsFrom(other, allowOtherFromArg)
63+
|
64+
// Call argument
65+
exists(DataFlowCall call, int i |
66+
other.(DataFlow::ParameterNode).isParameterOf(call.getStaticCallTarget(), i) and
67+
src.(ArgumentNode).argumentOf(call, i)
68+
) and
69+
allowOtherFromArg = true and
70+
allowFromArg = true
71+
or
72+
// Call return
73+
exists(DataFlowCall call, ReturnKind returnKind |
74+
other = getAnOutNode(call, returnKind) and
75+
src.(ReturnNode).getKind() = returnKind and
76+
call.getStaticCallTarget() = src.getEnclosingCallable()
77+
) and
78+
allowFromArg = false
79+
or
80+
// Local flow
81+
DataFlow::localFlowStep(src, other) and
82+
allowFromArg = allowOtherFromArg
83+
)
84+
or
85+
// Flow through global variable
86+
exists(StoreInstruction store, Variable var |
87+
store = src.asInstruction() and
88+
var = store.getDestinationAddress().(VariableAddressInstruction).getASTVariable() and
89+
this.flowsFromGlobal(var) and
90+
allowFromArg = true
91+
)
92+
}
93+
94+
private predicate flowsFromGlobal(GlobalOrNamespaceVariable var) {
95+
exists(LoadInstruction load |
96+
this.flowsFrom(DataFlow::instructionNode(load), _) and
97+
load.getSourceAddress().(VariableAddressInstruction).getASTVariable() = var
98+
)
99+
}
100+
}
101+
102+
/** Call through a function pointer. */
103+
private class DataSensitiveExprCall extends DataSensitiveCall {
104+
DataSensitiveExprCall() { not exists(this.getStaticCallTarget()) }
105+
106+
override DataFlow::Node getDispatchValue() { result.asInstruction() = this.getCallTarget() }
107+
108+
override Function resolve() {
109+
exists(FunctionInstruction fi |
110+
this.flowsFrom(DataFlow::instructionNode(fi), _) and
111+
result = fi.getFunctionSymbol()
112+
)
113+
}
114+
}
115+
116+
/** Call to a virtual function. */
117+
private class DataSensitiveOverriddenFunctionCall extends DataSensitiveCall {
118+
DataSensitiveOverriddenFunctionCall() {
119+
exists(this.getStaticCallTarget().(VirtualFunction).getAnOverridingFunction())
120+
}
121+
122+
override DataFlow::Node getDispatchValue() { result.asInstruction() = this.getThisArgument() }
123+
124+
override MemberFunction resolve() {
125+
exists(Class overridingClass |
126+
this.overrideMayAffectCall(overridingClass, result) and
127+
this.hasFlowFromCastFrom(overridingClass)
128+
)
129+
}
130+
131+
/**
132+
* Holds if `this` is a virtual function call whose static target is
133+
* overridden by `overridingFunction` in `overridingClass`.
134+
*/
135+
pragma[noinline]
136+
private predicate overrideMayAffectCall(Class overridingClass, MemberFunction overridingFunction) {
137+
overridingFunction.getAnOverriddenFunction+() = this.getStaticCallTarget().(VirtualFunction) and
138+
overridingFunction.getDeclaringType() = overridingClass
139+
}
140+
141+
/**
142+
* Holds if the qualifier of `this` has flow from an upcast from
143+
* `derivedClass`.
144+
*/
145+
pragma[noinline]
146+
private predicate hasFlowFromCastFrom(Class derivedClass) {
147+
exists(ConvertToBaseInstruction toBase |
148+
this.flowsFrom(DataFlow::instructionNode(toBase), _) and
149+
derivedClass = toBase.getDerivedClass()
150+
)
151+
}
152+
}
76153
}
77154

78155
/**

‎cpp/ql/test/library-tests/dataflow/dataflow-tests/dispatch.cpp‎

Lines changed: 86 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,3 +44,89 @@ void VirtualDispatch(Bottom *bottomPtr, Bottom &bottomRef) {
4444
sink(topRef.notSource2()); // no flow [FALSE POSITIVE]
4545
topRef.notSink(source()); // no flow [FALSE POSITIVE]
4646
}
47+
48+
Top *globalBottom, *globalMiddle;
49+
50+
Top *readGlobalBottom() {
51+
return globalBottom;
52+
}
53+
54+
void DispatchThroughGlobal() {
55+
sink(globalBottom->isSource1()); // flow [NOT DETECTED by AST]
56+
sink(globalMiddle->isSource1()); // no flow
57+
58+
sink(readGlobalBottom()->isSource1()); // flow [NOT DETECTED by AST]
59+
60+
globalBottom = new Bottom();
61+
globalMiddle = new Middle();
62+
}
63+
64+
Top *allocateBottom() {
65+
return new Bottom();
66+
}
67+
68+
void callSinkByPointer(Top *top) {
69+
top->isSink(source()); // flow [NOT DETECTED by AST]
70+
}
71+
72+
void callSinkByReference(Top &top) {
73+
top.isSink(source()); // flow [NOT DETECTED by AST]
74+
}
75+
76+
void globalVirtualDispatch() {
77+
callSinkByPointer(allocateBottom());
78+
callSinkByReference(*allocateBottom());
79+
80+
Top *x = allocateBottom();
81+
x->isSink(source()); // flow [NOT DETECTED by AST]
82+
}
83+
84+
Top *identity(Top *top) {
85+
return top;
86+
}
87+
88+
void callIdentityFunctions(Top *top, Bottom *bottom) {
89+
identity(bottom)->isSink(source()); // flow [NOT DETECTED]
90+
identity(top)->isSink(source()); // now flow
91+
}
92+
93+
using SinkFunctionType = void (*)(int);
94+
95+
void callSink(int x) {
96+
sink(x);
97+
}
98+
99+
SinkFunctionType returnCallSink() {
100+
return callSink;
101+
}
102+
103+
void testFunctionPointer(SinkFunctionType maybeCallSink, SinkFunctionType dontCallSink, bool b) {
104+
if (b) {
105+
maybeCallSink = returnCallSink();
106+
}
107+
maybeCallSink(source()); // flow [NOT DETECTED by AST]
108+
dontCallSink(source()); // no flow
109+
}
110+
111+
namespace virtual_inheritance {
112+
struct Top {
113+
virtual int isSource() { return 0; }
114+
};
115+
116+
struct Middle : virtual Top {
117+
int isSource() override { return source(); }
118+
};
119+
120+
struct Bottom : Middle {
121+
};
122+
123+
void VirtualDispatch(Bottom *bottomPtr, Bottom &bottomRef) {
124+
// Because the inheritance from `Top` is virtual, the following casts go
125+
// directly from `Bottom` to `Top`, skipping `Middle`. That means we don't
126+
// get flow from a `Middle` value to the call qualifier.
127+
Top *topPtr = bottomPtr, &topRef = bottomRef;
128+
129+
sink(topPtr->isSource()); // flow [NOT DETECTED]
130+
sink(topRef.isSource()); // flow [NOT DETECTED]
131+
}
132+
}

‎cpp/ql/test/library-tests/dataflow/dataflow-tests/test_diff.expected‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,8 +10,14 @@
1010
| dispatch.cpp:16:37:16:42 | dispatch.cpp:40:15:40:23 | IR only |
1111
| dispatch.cpp:22:37:22:42 | dispatch.cpp:31:16:31:24 | IR only |
1212
| dispatch.cpp:22:37:22:42 | dispatch.cpp:39:15:39:23 | IR only |
13+
| dispatch.cpp:22:37:22:42 | dispatch.cpp:55:22:55:30 | IR only |
14+
| dispatch.cpp:22:37:22:42 | dispatch.cpp:58:28:58:36 | IR only |
1315
| dispatch.cpp:33:18:33:23 | dispatch.cpp:23:38:23:38 | IR only |
1416
| dispatch.cpp:41:17:41:22 | dispatch.cpp:23:38:23:38 | IR only |
17+
| dispatch.cpp:69:15:69:20 | dispatch.cpp:23:38:23:38 | IR only |
18+
| dispatch.cpp:73:14:73:19 | dispatch.cpp:23:38:23:38 | IR only |
19+
| dispatch.cpp:81:13:81:18 | dispatch.cpp:23:38:23:38 | IR only |
20+
| dispatch.cpp:107:17:107:22 | dispatch.cpp:96:8:96:8 | IR only |
1521
| lambdas.cpp:8:10:8:15 | lambdas.cpp:14:3:14:6 | AST only |
1622
| lambdas.cpp:8:10:8:15 | lambdas.cpp:18:8:18:8 | AST only |
1723
| lambdas.cpp:8:10:8:15 | lambdas.cpp:21:3:21:6 | AST only |

‎cpp/ql/test/library-tests/dataflow/dataflow-tests/test_ir.expected‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,9 @@
1414
| dispatch.cpp:11:38:11:38 | x | dispatch.cpp:45:18:45:23 | call to source |
1515
| dispatch.cpp:23:38:23:38 | x | dispatch.cpp:33:18:33:23 | call to source |
1616
| dispatch.cpp:23:38:23:38 | x | dispatch.cpp:41:17:41:22 | call to source |
17+
| dispatch.cpp:23:38:23:38 | x | dispatch.cpp:69:15:69:20 | call to source |
18+
| dispatch.cpp:23:38:23:38 | x | dispatch.cpp:73:14:73:19 | call to source |
19+
| dispatch.cpp:23:38:23:38 | x | dispatch.cpp:81:13:81:18 | call to source |
1720
| dispatch.cpp:31:16:31:24 | call to isSource1 | dispatch.cpp:22:37:22:42 | call to source |
1821
| dispatch.cpp:32:16:32:24 | call to isSource2 | dispatch.cpp:16:37:16:42 | call to source |
1922
| dispatch.cpp:35:16:35:25 | call to notSource1 | dispatch.cpp:9:37:9:42 | call to source |
@@ -22,6 +25,9 @@
2225
| dispatch.cpp:40:15:40:23 | call to isSource2 | dispatch.cpp:16:37:16:42 | call to source |
2326
| dispatch.cpp:43:15:43:24 | call to notSource1 | dispatch.cpp:9:37:9:42 | call to source |
2427
| dispatch.cpp:44:15:44:24 | call to notSource2 | dispatch.cpp:10:37:10:42 | call to source |
28+
| dispatch.cpp:55:22:55:30 | call to isSource1 | dispatch.cpp:22:37:22:42 | call to source |
29+
| dispatch.cpp:58:28:58:36 | call to isSource1 | dispatch.cpp:22:37:22:42 | call to source |
30+
| dispatch.cpp:96:8:96:8 | x | dispatch.cpp:107:17:107:22 | call to source |
2531
| test.cpp:7:8:7:9 | t1 | test.cpp:6:12:6:17 | call to source |
2632
| test.cpp:9:8:9:9 | t1 | test.cpp:6:12:6:17 | call to source |
2733
| test.cpp:10:8:10:9 | t2 | test.cpp:6:12:6:17 | call to source |

0 commit comments

Comments
 (0)