Skip to content

Commit f4d0c5e

Browse files
jbjjbj
committed
C++ IR: Support for global virtual dispatch
The IR data flow library now supports virtual dispatch with a library that's similar to `security.TaintTracking`. In particular, it should have the same performance characteristics. The main difference is that non-recursive callers of `flowsFrom` now pass `_` instead of `true` for `boolean allowFromArg`. This change allows flow through `return` to actually work.
1 parent f7278d3 commit f4d0c5e

File tree

4 files changed

+190
-46
lines changed

4 files changed

+190
-46
lines changed

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

Lines changed: 115 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,125 @@ 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+
abstract DataFlow::Node getSrc();
4938

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-
)
39+
/** Gets a candidate target for this call. */
40+
cached
41+
abstract Function resolve();
42+
43+
/**
44+
* Whether `src` can flow to this call.
45+
*
46+
* Searches backwards from `getSrc()` to `src`. The `allowFromArg`
47+
* parameter is true when the search is allowed to continue backwards into
48+
* a parameter; non-recursive callers should pass `_` for `allowFromArg`.
49+
*/
50+
predicate flowsFrom(DataFlow::Node src, boolean allowFromArg) {
51+
src = this.getSrc() and allowFromArg = true
52+
or
53+
exists(DataFlow::Node other, boolean allowOtherFromArg |
54+
this.flowsFrom(other, allowOtherFromArg)
55+
|
56+
// Call argument
57+
exists(DataFlowCall call, int i |
58+
other.(DataFlow::ParameterNode).isParameterOf(call.getStaticCallTarget(), i) and
59+
src.(ArgumentNode).argumentOf(call, i)
60+
) and
61+
allowOtherFromArg = true and
62+
allowFromArg = true
63+
or
64+
// Call return
65+
exists(DataFlowCall call, ReturnKind returnKind |
66+
other = getAnOutNode(call, returnKind) and
67+
src.(ReturnNode).getKind() = returnKind and
68+
call.getStaticCallTarget() = src.getEnclosingCallable()
69+
) and
70+
allowFromArg = false
71+
or
72+
// Local flow
73+
DataFlow::localFlowStep(src, other) and
74+
allowFromArg = allowOtherFromArg
75+
)
76+
or
77+
// Flow through global variable
78+
exists(StoreInstruction store, Variable var |
79+
store = src.asInstruction() and
80+
var = store.getDestinationAddress().(VariableAddressInstruction).getASTVariable() and
81+
this.flowsFromGlobal(var) and
82+
allowFromArg = true
83+
)
84+
}
85+
86+
private predicate flowsFromGlobal(GlobalOrNamespaceVariable var) {
87+
exists(LoadInstruction load |
88+
this.flowsFrom(DataFlow::instructionNode(load), _) and
89+
load.getSourceAddress().(VariableAddressInstruction).getASTVariable() = var
90+
)
91+
}
92+
}
93+
94+
/** Call through a function pointer. */
95+
private class DataSensitiveExprCall extends DataSensitiveCall {
96+
DataSensitiveExprCall() { not exists(this.getStaticCallTarget()) }
97+
98+
override DataFlow::Node getSrc() { result.asInstruction() = this.getCallTarget() }
99+
100+
override Function resolve() {
101+
exists(FunctionInstruction fi |
102+
this.flowsFrom(DataFlow::instructionNode(fi), _) and
103+
result = fi.getFunctionSymbol()
104+
)
105+
}
106+
}
107+
108+
/** Call to a virtual function. */
109+
private class DataSensitiveOverriddenFunctionCall extends DataSensitiveCall {
110+
DataSensitiveOverriddenFunctionCall() {
111+
exists(this.getStaticCallTarget().(VirtualFunction).getAnOverridingFunction())
112+
}
113+
114+
override DataFlow::Node getSrc() { result.asInstruction() = this.getThisArgument() }
115+
116+
override MemberFunction resolve() {
117+
exists(Class overridingClass |
118+
this.overrideMayAffectCall(overridingClass, result) and
119+
this.hasFlowFromCastFrom(overridingClass)
120+
)
121+
}
122+
123+
/**
124+
* Holds if `this` is a virtual function call whose static target is
125+
* overridden by `overridingFunction` in `overridingClass`.
126+
*/
127+
pragma[noinline]
128+
private predicate overrideMayAffectCall(Class overridingClass, MemberFunction overridingFunction) {
129+
overridingFunction.getAnOverriddenFunction+() = this.getStaticCallTarget().(VirtualFunction) and
130+
overridingFunction.getDeclaringType() = overridingClass
131+
}
132+
133+
/**
134+
* Holds if the qualifier of `this` has flow from an upcast from
135+
* `derivedClass`.
136+
*/
137+
pragma[noinline]
138+
private predicate hasFlowFromCastFrom(Class derivedClass) {
139+
exists(ConvertToBaseInstruction toBase |
140+
this.flowsFrom(DataFlow::instructionNode(toBase), _) and
141+
derivedClass = toBase.getDerivedClass()
142+
)
143+
}
144+
}
76145
}
77146

78147
/**

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

Lines changed: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,3 +44,66 @@ 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+
}

‎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)