Skip to content

Commit 9ff894b

Browse files
MathiasVPMathiasVP
committed
C++: Add support for AST dataflow out of functions that take a smart pointer by value.
1 parent 8159098 commit 9ff894b

File tree

6 files changed

+118
-7
lines changed

6 files changed

+118
-7
lines changed

‎cpp/ql/src/semmle/code/cpp/dataflow/internal/DataFlowUtil.qll

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -199,6 +199,8 @@ class DefinitionByReferenceOrIteratorNode extends PartialDefinitionNode {
199199
this.getPartialDefinition() instanceof DefinitionByReference
200200
or
201201
this.getPartialDefinition() instanceof DefinitionByIterator
202+
or
203+
this.getPartialDefinition() instanceof DefinitionBySmartPointer
202204
)
203205
}
204206

@@ -330,6 +332,18 @@ class IteratorPartialDefinitionNode extends PartialDefinitionNode {
330332
override Node getPreUpdateNode() { pd.definesExpressions(_, result.asExpr()) }
331333
}
332334

335+
/**
336+
* INTERNAL: do not use.
337+
*
338+
* A synthetic data flow node used for flow into a collection when a smart pointer
339+
* write occurs in a callee.
340+
*/
341+
class SmartPointerPartialDefinitionNode extends PartialDefinitionNode {
342+
override SmartPointerPartialDefinition pd;
343+
344+
override Node getPreUpdateNode() { pd.definesExpressions(_, result.asExpr()) }
345+
}
346+
333347
/**
334348
* A post-update node on the `e->f` in `f(&e->f)` (and other forms).
335349
*/

‎cpp/ql/src/semmle/code/cpp/dataflow/internal/FlowVar.qll

Lines changed: 71 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ private import semmle.code.cpp.controlflow.SSA
77
private import semmle.code.cpp.dataflow.internal.SubBasicBlocks
88
private import semmle.code.cpp.dataflow.internal.AddressFlow
99
private import semmle.code.cpp.models.implementations.Iterator
10+
private import semmle.code.cpp.models.interfaces.PointerWrapper
1011

1112
/**
1213
* A conceptual variable that is assigned only once, like an SSA variable. This
@@ -243,6 +244,54 @@ private module PartialDefinitions {
243244
}
244245
}
245246

247+
class SmartPointerPartialDefinition extends PartialDefinition {
248+
Variable pointer;
249+
Expr innerDefinedExpr;
250+
251+
SmartPointerPartialDefinition() {
252+
exists(Expr convertedInner |
253+
not this instanceof Conversion and
254+
valueToUpdate(convertedInner, this.getFullyConverted(), node) and
255+
innerDefinedExpr = convertedInner.getUnconverted() and
256+
innerDefinedExpr = getAPointerWrapperAccess(pointer)
257+
)
258+
or
259+
// iterators passed by value without a copy constructor
260+
exists(Call call |
261+
call = node and
262+
call.getAnArgument() = innerDefinedExpr and
263+
innerDefinedExpr = this and
264+
this = getAPointerWrapperAccess(pointer) and
265+
not call instanceof OverloadedPointerDereferenceExpr
266+
)
267+
or
268+
// iterators passed by value with a copy constructor
269+
exists(Call call, ConstructorCall copy |
270+
copy.getTarget() instanceof CopyConstructor and
271+
call = node and
272+
call.getAnArgument() = copy and
273+
copy.getArgument(0) = getAPointerWrapperAccess(pointer) and
274+
innerDefinedExpr = this and
275+
this = copy and
276+
not call instanceof OverloadedPointerDereferenceExpr
277+
)
278+
}
279+
280+
deprecated override predicate partiallyDefines(Variable v) { v = pointer }
281+
282+
deprecated override predicate partiallyDefinesThis(ThisExpr e) { none() }
283+
284+
override predicate definesExpressions(Expr inner, Expr outer) {
285+
inner = innerDefinedExpr and
286+
outer = this
287+
}
288+
289+
override predicate partiallyDefinesVariableAt(Variable v, ControlFlowNode cfn) {
290+
v = pointer and
291+
cfn = node
292+
}
293+
}
294+
246295
/**
247296
* A partial definition that's a definition via an output iterator.
248297
*/
@@ -256,6 +305,15 @@ private module PartialDefinitions {
256305
class DefinitionByReference extends VariablePartialDefinition {
257306
DefinitionByReference() { exists(Call c | this = c.getAnArgument() or this = c.getQualifier()) }
258307
}
308+
309+
/**
310+
* A partial definition that's a definition via a smart pointer being passed into a function.
311+
*/
312+
class DefinitionBySmartPointer extends SmartPointerPartialDefinition {
313+
DefinitionBySmartPointer() {
314+
exists(Call c | this = c.getAnArgument() or this = c.getQualifier())
315+
}
316+
}
259317
}
260318

261319
import PartialDefinitions
@@ -296,7 +354,8 @@ module FlowVar_internal {
296354
// treating them as immutable, but for data flow it gives better results in
297355
// practice to make the variable synonymous with its contents.
298356
not v.getUnspecifiedType() instanceof ReferenceType and
299-
not v instanceof IteratorParameter
357+
not v instanceof IteratorParameter and
358+
not v instanceof PointerWrapperParameter
300359
}
301360

302361
/**
@@ -648,6 +707,8 @@ module FlowVar_internal {
648707
)
649708
or
650709
p instanceof IteratorParameter
710+
or
711+
p instanceof PointerWrapperParameter
651712
}
652713

653714
/**
@@ -832,10 +893,19 @@ module FlowVar_internal {
832893
)
833894
}
834895

896+
Call getAPointerWrapperAccess(Variable pointer) {
897+
pointer.getUnspecifiedType() instanceof PointerWrapper and
898+
[result.getQualifier(), result.getAnArgument()] = pointer.getAnAccess()
899+
}
900+
835901
class IteratorParameter extends Parameter {
836902
IteratorParameter() { this.getUnspecifiedType() instanceof Iterator }
837903
}
838904

905+
class PointerWrapperParameter extends Parameter {
906+
PointerWrapperParameter() { this.getUnspecifiedType() instanceof PointerWrapper }
907+
}
908+
839909
/**
840910
* Holds if `v` is initialized to have value `assignedExpr`.
841911
*/

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

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111
private import semmle.code.cpp.models.interfaces.DataFlow
1212
private import semmle.code.cpp.models.interfaces.Taint
1313
private import semmle.code.cpp.models.interfaces.Iterator
14+
private import semmle.code.cpp.models.interfaces.PointerWrapper
1415

1516
private module DataFlow {
1617
import semmle.code.cpp.dataflow.internal.DataFlowUtil
@@ -141,7 +142,10 @@ private predicate noFlowFromChildExpr(Expr e) {
141142
or
142143
e instanceof LogicalOrExpr
143144
or
144-
e instanceof Call
145+
// Allow taint from `operator*` on smart pointers.
146+
exists(Call call | e = call |
147+
not call.getTarget() = any(PointerWrapper wrapper).getAnUnwrapperFunction()
148+
)
145149
or
146150
e instanceof SizeofOperator
147151
or

‎cpp/ql/src/semmle/code/cpp/exprs/Call.qll

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -314,6 +314,7 @@ class OverloadedPointerDereferenceFunction extends Function {
314314
* T1 operator*(const T2 &);
315315
* T1 a; T2 b;
316316
* a = *b;
317+
* ```
317318
*/
318319
class OverloadedPointerDereferenceExpr extends FunctionCall {
319320
OverloadedPointerDereferenceExpr() {

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

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3223,24 +3223,30 @@
32233223
| smart_pointer.cpp:11:30:11:50 | call to make_shared | smart_pointer.cpp:12:11:12:11 | p | |
32243224
| smart_pointer.cpp:11:30:11:50 | call to make_shared | smart_pointer.cpp:13:10:13:10 | p | |
32253225
| smart_pointer.cpp:11:52:11:57 | call to source | smart_pointer.cpp:11:30:11:50 | call to make_shared | TAINT |
3226+
| smart_pointer.cpp:12:10:12:10 | call to operator* [post update] | smart_pointer.cpp:13:10:13:10 | p | |
32263227
| smart_pointer.cpp:12:11:12:11 | p | smart_pointer.cpp:12:10:12:10 | call to operator* | TAINT |
32273228
| smart_pointer.cpp:12:11:12:11 | ref arg p | smart_pointer.cpp:13:10:13:10 | p | |
32283229
| smart_pointer.cpp:17:32:17:54 | call to make_shared | smart_pointer.cpp:18:11:18:11 | p | |
32293230
| smart_pointer.cpp:17:32:17:54 | call to make_shared | smart_pointer.cpp:19:10:19:10 | p | |
3231+
| smart_pointer.cpp:18:10:18:10 | ref arg call to operator* | smart_pointer.cpp:19:10:19:10 | p | |
32303232
| smart_pointer.cpp:18:11:18:11 | p | smart_pointer.cpp:18:10:18:10 | call to operator* | TAINT |
32313233
| smart_pointer.cpp:18:11:18:11 | ref arg p | smart_pointer.cpp:19:10:19:10 | p | |
32323234
| smart_pointer.cpp:23:30:23:50 | call to make_unique | smart_pointer.cpp:24:11:24:11 | p | |
32333235
| smart_pointer.cpp:23:30:23:50 | call to make_unique | smart_pointer.cpp:25:10:25:10 | p | |
32343236
| smart_pointer.cpp:23:52:23:57 | call to source | smart_pointer.cpp:23:30:23:50 | call to make_unique | TAINT |
3237+
| smart_pointer.cpp:24:10:24:10 | call to operator* [post update] | smart_pointer.cpp:25:10:25:10 | p | |
32353238
| smart_pointer.cpp:24:11:24:11 | p | smart_pointer.cpp:24:10:24:10 | call to operator* | TAINT |
32363239
| smart_pointer.cpp:24:11:24:11 | ref arg p | smart_pointer.cpp:25:10:25:10 | p | |
32373240
| smart_pointer.cpp:29:32:29:54 | call to make_unique | smart_pointer.cpp:30:11:30:11 | p | |
32383241
| smart_pointer.cpp:29:32:29:54 | call to make_unique | smart_pointer.cpp:31:10:31:10 | p | |
3242+
| smart_pointer.cpp:30:10:30:10 | ref arg call to operator* | smart_pointer.cpp:31:10:31:10 | p | |
32393243
| smart_pointer.cpp:30:11:30:11 | p | smart_pointer.cpp:30:10:30:10 | call to operator* | TAINT |
32403244
| smart_pointer.cpp:30:11:30:11 | ref arg p | smart_pointer.cpp:31:10:31:10 | p | |
32413245
| smart_pointer.cpp:35:30:35:50 | call to make_shared | smart_pointer.cpp:37:6:37:6 | p | |
32423246
| smart_pointer.cpp:35:30:35:50 | call to make_shared | smart_pointer.cpp:38:10:38:10 | p | |
32433247
| smart_pointer.cpp:35:30:35:50 | call to make_shared | smart_pointer.cpp:39:11:39:11 | p | |
3248+
| smart_pointer.cpp:37:5:37:5 | call to operator* [post update] | smart_pointer.cpp:38:10:38:10 | p | |
3249+
| smart_pointer.cpp:37:5:37:5 | call to operator* [post update] | smart_pointer.cpp:39:11:39:11 | p | |
32443250
| smart_pointer.cpp:37:5:37:17 | ... = ... | smart_pointer.cpp:37:5:37:5 | call to operator* [post update] | |
32453251
| smart_pointer.cpp:37:6:37:6 | p | smart_pointer.cpp:37:5:37:5 | call to operator* | TAINT |
32463252
| smart_pointer.cpp:37:6:37:6 | ref arg p | smart_pointer.cpp:38:10:38:10 | p | |
@@ -3251,6 +3257,8 @@
32513257
| smart_pointer.cpp:43:29:43:51 | call to unique_ptr | smart_pointer.cpp:45:6:45:6 | p | |
32523258
| smart_pointer.cpp:43:29:43:51 | call to unique_ptr | smart_pointer.cpp:46:10:46:10 | p | |
32533259
| smart_pointer.cpp:43:29:43:51 | call to unique_ptr | smart_pointer.cpp:47:11:47:11 | p | |
3260+
| smart_pointer.cpp:45:5:45:5 | call to operator* [post update] | smart_pointer.cpp:46:10:46:10 | p | |
3261+
| smart_pointer.cpp:45:5:45:5 | call to operator* [post update] | smart_pointer.cpp:47:11:47:11 | p | |
32543262
| smart_pointer.cpp:45:5:45:17 | ... = ... | smart_pointer.cpp:45:5:45:5 | call to operator* [post update] | |
32553263
| smart_pointer.cpp:45:6:45:6 | p | smart_pointer.cpp:45:5:45:5 | call to operator* | TAINT |
32563264
| smart_pointer.cpp:45:6:45:6 | ref arg p | smart_pointer.cpp:46:10:46:10 | p | |
@@ -3268,7 +3276,21 @@
32683276
| smart_pointer.cpp:65:28:65:46 | call to make_unique | smart_pointer.cpp:67:10:67:10 | p | |
32693277
| smart_pointer.cpp:65:48:65:53 | call to source | smart_pointer.cpp:65:28:65:46 | call to make_unique | TAINT |
32703278
| smart_pointer.cpp:65:58:65:58 | 0 | smart_pointer.cpp:65:28:65:46 | call to make_unique | TAINT |
3279+
| smart_pointer.cpp:66:10:66:10 | p | smart_pointer.cpp:66:11:66:11 | call to operator-> | TAINT |
32713280
| smart_pointer.cpp:66:10:66:10 | ref arg p | smart_pointer.cpp:67:10:67:10 | p | |
3281+
| smart_pointer.cpp:67:10:67:10 | p | smart_pointer.cpp:67:11:67:11 | call to operator-> | TAINT |
3282+
| smart_pointer.cpp:70:37:70:39 | ptr | smart_pointer.cpp:70:37:70:39 | ptr | |
3283+
| smart_pointer.cpp:70:37:70:39 | ptr | smart_pointer.cpp:71:4:71:6 | ptr | |
3284+
| smart_pointer.cpp:71:3:71:3 | call to operator* [post update] | smart_pointer.cpp:70:37:70:39 | ptr | |
3285+
| smart_pointer.cpp:71:3:71:17 | ... = ... | smart_pointer.cpp:71:3:71:3 | call to operator* [post update] | |
3286+
| smart_pointer.cpp:71:4:71:6 | ptr | smart_pointer.cpp:71:3:71:3 | call to operator* | TAINT |
3287+
| smart_pointer.cpp:71:4:71:6 | ref arg ptr | smart_pointer.cpp:70:37:70:39 | ptr | |
3288+
| smart_pointer.cpp:71:10:71:15 | call to source | smart_pointer.cpp:71:3:71:17 | ... = ... | |
3289+
| smart_pointer.cpp:75:26:75:33 | call to shared_ptr | smart_pointer.cpp:76:13:76:13 | p | |
3290+
| smart_pointer.cpp:75:26:75:33 | call to shared_ptr | smart_pointer.cpp:77:9:77:9 | p | |
3291+
| smart_pointer.cpp:76:13:76:13 | call to shared_ptr [post update] | smart_pointer.cpp:77:9:77:9 | p | |
3292+
| smart_pointer.cpp:76:13:76:13 | p | smart_pointer.cpp:76:13:76:13 | call to shared_ptr | |
3293+
| smart_pointer.cpp:77:9:77:9 | p | smart_pointer.cpp:77:8:77:8 | call to operator* | TAINT |
32723294
| standalone_iterators.cpp:39:45:39:51 | source1 | standalone_iterators.cpp:39:45:39:51 | source1 | |
32733295
| standalone_iterators.cpp:39:45:39:51 | source1 | standalone_iterators.cpp:40:11:40:17 | source1 | |
32743296
| standalone_iterators.cpp:39:45:39:51 | source1 | standalone_iterators.cpp:41:12:41:18 | source1 | |

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

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -35,16 +35,16 @@ void test_reverse_taint_shared() {
3535
std::shared_ptr<int> p = std::make_shared<int>();
3636

3737
*p = source();
38-
sink(p); // $ MISSING: ast,ir
39-
sink(*p); // $ MISSING: ast,ir
38+
sink(p); // $ ast MISSING: ir
39+
sink(*p); // $ ast MISSING: ir
4040
}
4141

4242
void test_reverse_taint_unique() {
4343
std::unique_ptr<int> p = std::unique_ptr<int>();
4444

4545
*p = source();
46-
sink(p); // $ MISSING: ast,ir
47-
sink(*p); // $ MISSING: ast,ir
46+
sink(p); // $ ast MISSING: ir
47+
sink(*p); // $ ast MISSING: ir
4848
}
4949

5050
void test_shared_get() {
@@ -74,5 +74,5 @@ void getNumber(std::shared_ptr<int> ptr) {
7474
int test_from_issue_5190() {
7575
std::shared_ptr<int> p(new int);
7676
getNumber(p);
77-
sink(*p); // $ MISSING: ast,ir
77+
sink(*p); // $ ast MISSING: ir
7878
}

0 commit comments

Comments
 (0)