Skip to content

Commit 8a0089a

Browse files
authored
Merge pull request #2672 from geoffw0/qualifierflow
CPP: Support taint flow in and out of qualifiers
2 parents 95f78e7 + ccf268d commit 8a0089a

File tree

7 files changed

+222
-13
lines changed

7 files changed

+222
-13
lines changed

cpp/ql/src/semmle/code/cpp/dataflow/internal/TaintTrackingUtil.qll

Lines changed: 55 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -68,9 +68,11 @@ predicate localAdditionalTaintStep(DataFlow::Node nodeFrom, DataFlow::Node nodeT
6868
)
6969
or
7070
// Taint can flow through modeled functions
71+
exprToExprStep(nodeFrom.asExpr(), nodeTo.asExpr())
72+
or
7173
exprToDefinitionByReferenceStep(nodeFrom.asExpr(), nodeTo.asDefiningArgument())
7274
or
73-
exprToExprStep(nodeFrom.asExpr(), nodeTo.asExpr())
75+
exprToPartialDefinitionStep(nodeFrom.asExpr(), nodeTo.asPartialDefinition())
7476
}
7577

7678
/**
@@ -133,19 +135,30 @@ private predicate exprToExprStep(Expr exprIn, Expr exprOut) {
133135
)
134136
)
135137
or
136-
exists(TaintFunction f, Call call, FunctionOutput outModel |
138+
exists(TaintFunction f, Call call, FunctionInput inModel, FunctionOutput outModel |
137139
call.getTarget() = f and
138-
exprOut = call and
139-
outModel.isReturnValueDeref() and
140-
exists(int argInIndex, FunctionInput inModel | f.hasTaintFlow(inModel, outModel) |
141-
inModel.isParameterDeref(argInIndex) and
142-
exprIn = call.getArgument(argInIndex)
140+
(
141+
exprOut = call and
142+
outModel.isReturnValueDeref()
143143
or
144-
inModel.isParameterDeref(argInIndex) and
145-
call.passesByReference(argInIndex, exprIn)
144+
exprOut = call and
145+
outModel.isReturnValue()
146+
) and
147+
f.hasTaintFlow(inModel, outModel) and
148+
(
149+
exists(int argInIndex |
150+
inModel.isParameterDeref(argInIndex) and
151+
exprIn = call.getArgument(argInIndex)
152+
or
153+
inModel.isParameterDeref(argInIndex) and
154+
call.passesByReference(argInIndex, exprIn)
155+
or
156+
inModel.isParameter(argInIndex) and
157+
exprIn = call.getArgument(argInIndex)
158+
)
146159
or
147-
inModel.isParameter(argInIndex) and
148-
exprIn = call.getArgument(argInIndex)
160+
inModel.isQualifierObject() and
161+
exprIn = call.getQualifier()
149162
)
150163
)
151164
}
@@ -163,11 +176,40 @@ private predicate exprToDefinitionByReferenceStep(Expr exprIn, Expr argOut) {
163176
)
164177
)
165178
or
166-
exists(TaintFunction f, Call call, FunctionOutput outModel, int argOutIndex |
179+
exists(
180+
TaintFunction f, Call call, FunctionInput inModel, FunctionOutput outModel, int argOutIndex
181+
|
167182
call.getTarget() = f and
168183
argOut = call.getArgument(argOutIndex) and
169184
outModel.isParameterDeref(argOutIndex) and
170-
exists(int argInIndex, FunctionInput inModel | f.hasTaintFlow(inModel, outModel) |
185+
f.hasTaintFlow(inModel, outModel) and
186+
(
187+
exists(int argInIndex |
188+
inModel.isParameterDeref(argInIndex) and
189+
exprIn = call.getArgument(argInIndex)
190+
or
191+
inModel.isParameterDeref(argInIndex) and
192+
call.passesByReference(argInIndex, exprIn)
193+
or
194+
inModel.isParameter(argInIndex) and
195+
exprIn = call.getArgument(argInIndex)
196+
)
197+
or
198+
inModel.isQualifierObject() and
199+
exprIn = call.getQualifier()
200+
)
201+
)
202+
}
203+
204+
private predicate exprToPartialDefinitionStep(Expr exprIn, Expr exprOut) {
205+
exists(TaintFunction f, Call call, FunctionInput inModel, FunctionOutput outModel |
206+
call.getTarget() = f and
207+
(
208+
exprOut = call.getQualifier() and
209+
outModel.isQualifierObject()
210+
) and
211+
f.hasTaintFlow(inModel, outModel) and
212+
exists(int argInIndex |
171213
inModel.isParameterDeref(argInIndex) and
172214
exprIn = call.getArgument(argInIndex)
173215
or

cpp/ql/test/library-tests/dataflow/taint-tests/TaintTestCommon.qll

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import cpp
22
import semmle.code.cpp.dataflow.TaintTracking
3+
import semmle.code.cpp.models.interfaces.Taint
34

45
/** Common data flow configuration to be used by tests. */
56
class TestAllocationConfig extends TaintTracking::Configuration {
@@ -25,3 +26,39 @@ class TestAllocationConfig extends TaintTracking::Configuration {
2526
barrier.asExpr().(VariableAccess).getTarget().hasName("sanitizer")
2627
}
2728
}
29+
30+
class SetMemberFunction extends TaintFunction {
31+
SetMemberFunction() { this.hasName("setMember") }
32+
33+
override predicate hasTaintFlow(FunctionInput input, FunctionOutput output) {
34+
input.isParameter(0) and
35+
output.isQualifierObject()
36+
}
37+
}
38+
39+
class GetMemberFunction extends TaintFunction {
40+
GetMemberFunction() { this.hasName("getMember") }
41+
42+
override predicate hasTaintFlow(FunctionInput input, FunctionOutput output) {
43+
input.isQualifierObject() and
44+
output.isReturnValue()
45+
}
46+
}
47+
48+
class SetStringFunction extends TaintFunction {
49+
SetStringFunction() { this.hasName("setString") }
50+
51+
override predicate hasTaintFlow(FunctionInput input, FunctionOutput output) {
52+
input.isParameterDeref(0) and
53+
output.isQualifierObject()
54+
}
55+
}
56+
57+
class GetStringFunction extends TaintFunction {
58+
GetStringFunction() { this.hasName("getString") }
59+
60+
override predicate hasTaintFlow(FunctionInput input, FunctionOutput output) {
61+
input.isQualifierObject() and
62+
output.isReturnValueDeref()
63+
}
64+
}

cpp/ql/test/library-tests/dataflow/taint-tests/localTaint.expected

Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -347,3 +347,62 @@
347347
| taint.cpp:390:6:390:11 | call to wcsdup | taint.cpp:390:2:390:28 | ... = ... | |
348348
| taint.cpp:390:6:390:11 | call to wcsdup | taint.cpp:392:7:392:7 | b | |
349349
| taint.cpp:390:13:390:27 | hello, world | taint.cpp:390:6:390:11 | call to wcsdup | TAINT |
350+
| taint.cpp:417:13:417:14 | call to MyClass2 | taint.cpp:420:7:420:7 | a | |
351+
| taint.cpp:417:13:417:14 | call to MyClass2 | taint.cpp:421:7:421:7 | a | |
352+
| taint.cpp:417:13:417:14 | call to MyClass2 | taint.cpp:422:2:422:2 | a | |
353+
| taint.cpp:417:13:417:14 | call to MyClass2 | taint.cpp:423:7:423:7 | a | |
354+
| taint.cpp:417:13:417:14 | call to MyClass2 | taint.cpp:424:7:424:7 | a | |
355+
| taint.cpp:417:19:417:20 | call to MyClass2 | taint.cpp:426:7:426:7 | b | |
356+
| taint.cpp:417:19:417:20 | call to MyClass2 | taint.cpp:427:7:427:7 | b | |
357+
| taint.cpp:417:19:417:20 | call to MyClass2 | taint.cpp:428:2:428:2 | b | |
358+
| taint.cpp:417:19:417:20 | call to MyClass2 | taint.cpp:429:7:429:7 | b | |
359+
| taint.cpp:417:19:417:20 | call to MyClass2 | taint.cpp:430:7:430:7 | b | |
360+
| taint.cpp:417:19:417:20 | call to MyClass2 | taint.cpp:431:7:431:7 | b | |
361+
| taint.cpp:418:13:418:15 | call to MyClass3 | taint.cpp:443:7:443:7 | d | |
362+
| taint.cpp:418:13:418:15 | call to MyClass3 | taint.cpp:444:7:444:7 | d | |
363+
| taint.cpp:418:13:418:15 | call to MyClass3 | taint.cpp:445:2:445:2 | d | |
364+
| taint.cpp:418:13:418:15 | call to MyClass3 | taint.cpp:446:7:446:7 | d | |
365+
| taint.cpp:418:13:418:15 | call to MyClass3 | taint.cpp:447:7:447:7 | d | |
366+
| taint.cpp:421:7:421:7 | a [post update] | taint.cpp:422:2:422:2 | a | |
367+
| taint.cpp:421:7:421:7 | a [post update] | taint.cpp:423:7:423:7 | a | |
368+
| taint.cpp:421:7:421:7 | a [post update] | taint.cpp:424:7:424:7 | a | |
369+
| taint.cpp:422:2:422:2 | a [post update] | taint.cpp:423:7:423:7 | a | |
370+
| taint.cpp:422:2:422:2 | a [post update] | taint.cpp:424:7:424:7 | a | |
371+
| taint.cpp:427:7:427:7 | b [post update] | taint.cpp:428:2:428:2 | b | |
372+
| taint.cpp:427:7:427:7 | b [post update] | taint.cpp:429:7:429:7 | b | |
373+
| taint.cpp:427:7:427:7 | b [post update] | taint.cpp:430:7:430:7 | b | |
374+
| taint.cpp:427:7:427:7 | b [post update] | taint.cpp:431:7:431:7 | b | |
375+
| taint.cpp:428:2:428:2 | b [post update] | taint.cpp:429:7:429:7 | b | |
376+
| taint.cpp:428:2:428:2 | b [post update] | taint.cpp:430:7:430:7 | b | |
377+
| taint.cpp:428:2:428:2 | b [post update] | taint.cpp:431:7:431:7 | b | |
378+
| taint.cpp:428:2:428:20 | ... = ... | taint.cpp:430:9:430:14 | member | |
379+
| taint.cpp:428:13:428:18 | call to source | taint.cpp:428:2:428:20 | ... = ... | |
380+
| taint.cpp:433:6:433:20 | call to MyClass2 | taint.cpp:433:6:433:20 | new | |
381+
| taint.cpp:433:6:433:20 | new | taint.cpp:433:2:433:20 | ... = ... | |
382+
| taint.cpp:433:6:433:20 | new | taint.cpp:435:7:435:7 | c | |
383+
| taint.cpp:433:6:433:20 | new | taint.cpp:436:7:436:7 | c | |
384+
| taint.cpp:433:6:433:20 | new | taint.cpp:437:2:437:2 | c | |
385+
| taint.cpp:433:6:433:20 | new | taint.cpp:438:7:438:7 | c | |
386+
| taint.cpp:433:6:433:20 | new | taint.cpp:439:7:439:7 | c | |
387+
| taint.cpp:433:6:433:20 | new | taint.cpp:441:9:441:9 | c | |
388+
| taint.cpp:435:7:435:7 | ref arg c | taint.cpp:436:7:436:7 | c | |
389+
| taint.cpp:435:7:435:7 | ref arg c | taint.cpp:437:2:437:2 | c | |
390+
| taint.cpp:435:7:435:7 | ref arg c | taint.cpp:438:7:438:7 | c | |
391+
| taint.cpp:435:7:435:7 | ref arg c | taint.cpp:439:7:439:7 | c | |
392+
| taint.cpp:435:7:435:7 | ref arg c | taint.cpp:441:9:441:9 | c | |
393+
| taint.cpp:436:7:436:7 | c [post update] | taint.cpp:437:2:437:2 | c | |
394+
| taint.cpp:436:7:436:7 | c [post update] | taint.cpp:438:7:438:7 | c | |
395+
| taint.cpp:436:7:436:7 | c [post update] | taint.cpp:439:7:439:7 | c | |
396+
| taint.cpp:436:7:436:7 | c [post update] | taint.cpp:441:9:441:9 | c | |
397+
| taint.cpp:437:2:437:2 | c [post update] | taint.cpp:438:7:438:7 | c | |
398+
| taint.cpp:437:2:437:2 | c [post update] | taint.cpp:439:7:439:7 | c | |
399+
| taint.cpp:437:2:437:2 | c [post update] | taint.cpp:441:9:441:9 | c | |
400+
| taint.cpp:438:7:438:7 | ref arg c | taint.cpp:439:7:439:7 | c | |
401+
| taint.cpp:438:7:438:7 | ref arg c | taint.cpp:441:9:441:9 | c | |
402+
| taint.cpp:439:7:439:7 | c [post update] | taint.cpp:441:9:441:9 | c | |
403+
| taint.cpp:441:9:441:9 | c | taint.cpp:441:2:441:9 | delete | TAINT |
404+
| taint.cpp:444:7:444:7 | d [post update] | taint.cpp:445:2:445:2 | d | |
405+
| taint.cpp:444:7:444:7 | d [post update] | taint.cpp:446:7:446:7 | d | |
406+
| taint.cpp:444:7:444:7 | d [post update] | taint.cpp:447:7:447:7 | d | |
407+
| taint.cpp:445:2:445:2 | d [post update] | taint.cpp:446:7:446:7 | d | |
408+
| taint.cpp:445:2:445:2 | d [post update] | taint.cpp:447:7:447:7 | d | |

cpp/ql/test/library-tests/dataflow/taint-tests/taint.cpp

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -391,3 +391,58 @@ void test_wcsdup(wchar_t *source)
391391
sink(a); // tainted
392392
sink(b);
393393
}
394+
395+
// --- qualifiers ---
396+
397+
class MyClass2 {
398+
public:
399+
MyClass2(int value);
400+
void setMember(int value);
401+
int getMember();
402+
403+
int member;
404+
};
405+
406+
class MyClass3 {
407+
public:
408+
MyClass3(const char *string);
409+
void setString(const char *string);
410+
const char *getString();
411+
412+
const char *buffer;
413+
};
414+
415+
void test_qualifiers()
416+
{
417+
MyClass2 a(0), b(0), *c;
418+
MyClass3 d("");
419+
420+
sink(a);
421+
sink(a.getMember());
422+
a.setMember(source());
423+
sink(a); // tainted
424+
sink(a.getMember()); // tainted
425+
426+
sink(b);
427+
sink(b.getMember());
428+
b.member = source();
429+
sink(b); // tainted
430+
sink(b.member); // tainted
431+
sink(b.getMember());
432+
433+
c = new MyClass2(0);
434+
435+
sink(c);
436+
sink(c->getMember());
437+
c->setMember(source());
438+
sink(c); // tainted (deref)
439+
sink(c->getMember()); // tainted
440+
441+
delete c;
442+
443+
sink(d);
444+
sink(d.getString());
445+
d.setString(strings::source());
446+
sink(d); // tainted
447+
sink(d.getString()); // tainted
448+
}

cpp/ql/test/library-tests/dataflow/taint-tests/taint.expected

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -39,3 +39,10 @@
3939
| taint.cpp:352:7:352:7 | b | taint.cpp:330:6:330:11 | call to source |
4040
| taint.cpp:372:7:372:7 | a | taint.cpp:365:24:365:29 | source |
4141
| taint.cpp:391:7:391:7 | a | taint.cpp:385:27:385:32 | source |
42+
| taint.cpp:423:7:423:7 | a | taint.cpp:422:14:422:19 | call to source |
43+
| taint.cpp:424:9:424:17 | call to getMember | taint.cpp:422:14:422:19 | call to source |
44+
| taint.cpp:430:9:430:14 | member | taint.cpp:428:13:428:18 | call to source |
45+
| taint.cpp:438:7:438:7 | c | taint.cpp:437:15:437:20 | call to source |
46+
| taint.cpp:439:10:439:18 | call to getMember | taint.cpp:437:15:437:20 | call to source |
47+
| taint.cpp:446:7:446:7 | d | taint.cpp:445:14:445:28 | call to source |
48+
| taint.cpp:447:9:447:17 | call to getString | taint.cpp:445:14:445:28 | call to source |

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

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,3 +26,11 @@
2626
| taint.cpp:352:7:352:7 | taint.cpp:330:6:330:11 | AST only |
2727
| taint.cpp:372:7:372:7 | taint.cpp:365:24:365:29 | AST only |
2828
| taint.cpp:391:7:391:7 | taint.cpp:385:27:385:32 | AST only |
29+
| taint.cpp:423:7:423:7 | taint.cpp:422:14:422:19 | AST only |
30+
| taint.cpp:424:9:424:17 | taint.cpp:422:14:422:19 | AST only |
31+
| taint.cpp:429:7:429:7 | taint.cpp:428:13:428:18 | IR only |
32+
| taint.cpp:430:9:430:14 | taint.cpp:428:13:428:18 | AST only |
33+
| taint.cpp:438:7:438:7 | taint.cpp:437:15:437:20 | AST only |
34+
| taint.cpp:439:10:439:18 | taint.cpp:437:15:437:20 | AST only |
35+
| taint.cpp:446:7:446:7 | taint.cpp:445:14:445:28 | AST only |
36+
| taint.cpp:447:9:447:17 | taint.cpp:445:14:445:28 | AST only |

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,3 +15,4 @@
1515
| taint.cpp:291:7:291:7 | y | taint.cpp:275:6:275:11 | call to source |
1616
| taint.cpp:337:7:337:7 | t | taint.cpp:330:6:330:11 | call to source |
1717
| taint.cpp:350:7:350:7 | t | taint.cpp:330:6:330:11 | call to source |
18+
| taint.cpp:429:7:429:7 | b | taint.cpp:428:13:428:18 | call to source |

0 commit comments

Comments
 (0)