Skip to content

Commit 833f5b0

Browse files
geoffw0geoffw0
committed
C++: Add flow through assignment operators.
1 parent b9a6558 commit 833f5b0

File tree

6 files changed

+60
-8
lines changed

6 files changed

+60
-8
lines changed

‎cpp/ql/src/semmle/code/cpp/MemberFunction.qll

Lines changed: 24 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -467,7 +467,7 @@ class ConversionOperator extends MemberFunction, ImplicitConversionFunction {
467467
* takes exactly one parameter of type `T`, `T&`, `const T&`, `volatile
468468
* T&`, or `const volatile T&`.
469469
*/
470-
class CopyAssignmentOperator extends Operator {
470+
class CopyAssignmentOperator extends Operator,TaintFunction {
471471
CopyAssignmentOperator() {
472472
hasName("operator=") and
473473
(
@@ -482,6 +482,17 @@ class CopyAssignmentOperator extends Operator {
482482
}
483483

484484
override string getCanonicalQLClass() { result = "CopyAssignmentOperator" }
485+
486+
override predicate hasTaintFlow(FunctionInput input, FunctionOutput output) {
487+
// taint flow from argument to self
488+
input.isParameterDeref(0) and
489+
output.isQualifierObject()
490+
or
491+
// taint flow from argument to return value
492+
input.isParameterDeref(0) and
493+
output.isReturnValueDeref()
494+
// TODO: it would be more accurate to model copy assignment as data flow
495+
}
485496
}
486497

487498
/**
@@ -499,7 +510,7 @@ class CopyAssignmentOperator extends Operator {
499510
* takes exactly one parameter of type `T&&`, `const T&&`, `volatile T&&`,
500511
* or `const volatile T&&`.
501512
*/
502-
class MoveAssignmentOperator extends Operator {
513+
class MoveAssignmentOperator extends Operator, TaintFunction {
503514
MoveAssignmentOperator() {
504515
hasName("operator=") and
505516
hasMoveSignature(this) and
@@ -508,4 +519,15 @@ class MoveAssignmentOperator extends Operator {
508519
}
509520

510521
override string getCanonicalQLClass() { result = "MoveAssignmentOperator" }
522+
523+
override predicate hasTaintFlow(FunctionInput input, FunctionOutput output) {
524+
// taint flow from argument to self
525+
input.isParameterDeref(0) and
526+
output.isQualifierObject()
527+
or
528+
// taint flow from argument to return value
529+
input.isParameterDeref(0) and
530+
output.isReturnValueDeref()
531+
// TODO: it would be more accurate to model move assignment as data flow
532+
}
511533
}

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

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,7 @@ void test_copyableclass()
3939
sink(s1); // tainted
4040
sink(s2); // tainted
4141
sink(s3); // tainted
42-
sink(s4); // tainted [NOT DETECTED]
42+
sink(s4); // tainted
4343
}
4444

4545
{
@@ -62,7 +62,7 @@ void test_copyableclass()
6262
s2 = MyCopyableClass(source());
6363

6464
sink(s1); // tainted
65-
sink(s2); // tainted [NOT DETECTED]
66-
sink(s3 = source()); // tainted [NOT DETECTED]
65+
sink(s2); // tainted
66+
sink(s3 = source()); // tainted
6767
}
6868
}

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

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,8 @@
2222
| copyableclass.cpp:23:19:23:20 | call to MyCopyableClass | copyableclass.cpp:29:8:29:9 | s4 | |
2323
| copyableclass.cpp:24:3:24:4 | ref arg s4 | copyableclass.cpp:29:8:29:9 | s4 | |
2424
| copyableclass.cpp:24:8:24:8 | 1 | copyableclass.cpp:24:8:24:8 | call to MyCopyableClass | TAINT |
25+
| copyableclass.cpp:24:8:24:8 | call to MyCopyableClass | copyableclass.cpp:24:3:24:4 | ref arg s4 | TAINT |
26+
| copyableclass.cpp:24:8:24:8 | call to MyCopyableClass | copyableclass.cpp:24:6:24:6 | call to operator= | TAINT |
2527
| copyableclass.cpp:33:22:33:27 | call to source | copyableclass.cpp:33:22:33:30 | call to MyCopyableClass | TAINT |
2628
| copyableclass.cpp:33:22:33:30 | call to MyCopyableClass | copyableclass.cpp:35:22:35:23 | s1 | |
2729
| copyableclass.cpp:33:22:33:30 | call to MyCopyableClass | copyableclass.cpp:39:8:39:9 | s1 | |
@@ -33,6 +35,8 @@
3335
| copyableclass.cpp:36:19:36:20 | call to MyCopyableClass | copyableclass.cpp:42:8:42:9 | s4 | |
3436
| copyableclass.cpp:37:3:37:4 | ref arg s4 | copyableclass.cpp:42:8:42:9 | s4 | |
3537
| copyableclass.cpp:37:8:37:13 | call to source | copyableclass.cpp:37:8:37:15 | call to MyCopyableClass | TAINT |
38+
| copyableclass.cpp:37:8:37:15 | call to MyCopyableClass | copyableclass.cpp:37:3:37:4 | ref arg s4 | TAINT |
39+
| copyableclass.cpp:37:8:37:15 | call to MyCopyableClass | copyableclass.cpp:37:6:37:6 | call to operator= | TAINT |
3640
| copyableclass.cpp:46:19:46:20 | call to MyCopyableClass | copyableclass.cpp:47:24:47:25 | s1 | |
3741
| copyableclass.cpp:46:19:46:20 | call to MyCopyableClass | copyableclass.cpp:48:22:48:23 | s1 | |
3842
| copyableclass.cpp:46:19:46:20 | call to MyCopyableClass | copyableclass.cpp:50:8:50:9 | s1 | |
@@ -44,14 +48,20 @@
4448
| copyableclass.cpp:49:19:49:20 | call to MyCopyableClass | copyableclass.cpp:50:3:50:4 | s4 | |
4549
| copyableclass.cpp:49:19:49:20 | call to MyCopyableClass | copyableclass.cpp:55:8:55:9 | s4 | |
4650
| copyableclass.cpp:50:3:50:4 | ref arg s4 | copyableclass.cpp:55:8:55:9 | s4 | |
51+
| copyableclass.cpp:50:8:50:9 | s1 | copyableclass.cpp:50:3:50:4 | ref arg s4 | TAINT |
52+
| copyableclass.cpp:50:8:50:9 | s1 | copyableclass.cpp:50:6:50:6 | call to operator= | TAINT |
4753
| copyableclass.cpp:59:23:59:48 | call to MyCopyableClass | copyableclass.cpp:64:8:64:9 | s1 | |
4854
| copyableclass.cpp:59:40:59:45 | call to source | copyableclass.cpp:59:23:59:48 | call to MyCopyableClass | TAINT |
4955
| copyableclass.cpp:60:19:60:20 | call to MyCopyableClass | copyableclass.cpp:62:3:62:4 | s2 | |
5056
| copyableclass.cpp:60:19:60:20 | call to MyCopyableClass | copyableclass.cpp:65:8:65:9 | s2 | |
5157
| copyableclass.cpp:61:19:61:20 | call to MyCopyableClass | copyableclass.cpp:66:8:66:9 | s3 | |
5258
| copyableclass.cpp:62:3:62:4 | ref arg s2 | copyableclass.cpp:65:8:65:9 | s2 | |
59+
| copyableclass.cpp:62:8:62:32 | call to MyCopyableClass | copyableclass.cpp:62:3:62:4 | ref arg s2 | TAINT |
60+
| copyableclass.cpp:62:8:62:32 | call to MyCopyableClass | copyableclass.cpp:62:6:62:6 | call to operator= | TAINT |
5361
| copyableclass.cpp:62:24:62:29 | call to source | copyableclass.cpp:62:8:62:32 | call to MyCopyableClass | TAINT |
5462
| copyableclass.cpp:66:13:66:18 | call to source | copyableclass.cpp:66:13:66:20 | call to MyCopyableClass | TAINT |
63+
| copyableclass.cpp:66:13:66:20 | call to MyCopyableClass | copyableclass.cpp:66:8:66:9 | ref arg s3 | TAINT |
64+
| copyableclass.cpp:66:13:66:20 | call to MyCopyableClass | copyableclass.cpp:66:11:66:11 | call to operator= | TAINT |
5565
| file://:0:0:0:0 | p#0 | file://:0:0:0:0 | p#0 | |
5666
| file://:0:0:0:0 | p#0 | file://:0:0:0:0 | p#0 | |
5767
| file://:0:0:0:0 | p#0 | file://:0:0:0:0 | p#0 | |
@@ -217,6 +227,8 @@
217227
| movableclass.cpp:29:18:29:19 | call to MyMovableClass | movableclass.cpp:34:8:34:9 | s3 | |
218228
| movableclass.cpp:30:3:30:4 | ref arg s3 | movableclass.cpp:34:8:34:9 | s3 | |
219229
| movableclass.cpp:30:8:30:8 | 1 | movableclass.cpp:30:8:30:8 | call to MyMovableClass | TAINT |
230+
| movableclass.cpp:30:8:30:8 | call to MyMovableClass | movableclass.cpp:30:3:30:4 | ref arg s3 | TAINT |
231+
| movableclass.cpp:30:8:30:8 | call to MyMovableClass | movableclass.cpp:30:6:30:6 | call to operator= | TAINT |
220232
| movableclass.cpp:38:21:38:26 | call to source | movableclass.cpp:38:21:38:29 | call to MyMovableClass | TAINT |
221233
| movableclass.cpp:38:21:38:29 | call to MyMovableClass | movableclass.cpp:43:8:43:9 | s1 | |
222234
| movableclass.cpp:39:22:39:30 | call to MyMovableClass | movableclass.cpp:44:8:44:9 | s2 | |
@@ -225,18 +237,24 @@
225237
| movableclass.cpp:40:18:40:19 | call to MyMovableClass | movableclass.cpp:45:8:45:9 | s3 | |
226238
| movableclass.cpp:41:3:41:4 | ref arg s3 | movableclass.cpp:45:8:45:9 | s3 | |
227239
| movableclass.cpp:41:8:41:13 | call to source | movableclass.cpp:41:8:41:15 | call to MyMovableClass | TAINT |
240+
| movableclass.cpp:41:8:41:15 | call to MyMovableClass | movableclass.cpp:41:3:41:4 | ref arg s3 | TAINT |
241+
| movableclass.cpp:41:8:41:15 | call to MyMovableClass | movableclass.cpp:41:6:41:6 | call to operator= | TAINT |
228242
| movableclass.cpp:49:22:49:46 | call to MyMovableClass | movableclass.cpp:53:8:53:9 | s1 | |
229243
| movableclass.cpp:49:38:49:43 | call to source | movableclass.cpp:49:22:49:46 | call to MyMovableClass | TAINT |
230244
| movableclass.cpp:50:18:50:19 | call to MyMovableClass | movableclass.cpp:51:3:51:4 | s2 | |
231245
| movableclass.cpp:50:18:50:19 | call to MyMovableClass | movableclass.cpp:54:8:54:9 | s2 | |
232246
| movableclass.cpp:51:3:51:4 | ref arg s2 | movableclass.cpp:54:8:54:9 | s2 | |
247+
| movableclass.cpp:51:8:51:31 | call to MyMovableClass | movableclass.cpp:51:3:51:4 | ref arg s2 | TAINT |
248+
| movableclass.cpp:51:8:51:31 | call to MyMovableClass | movableclass.cpp:51:6:51:6 | call to operator= | TAINT |
233249
| movableclass.cpp:51:23:51:28 | call to source | movableclass.cpp:51:8:51:31 | call to MyMovableClass | TAINT |
234250
| movableclass.cpp:58:21:58:32 | call to getUnTainted | movableclass.cpp:58:21:58:35 | call to MyMovableClass | |
235251
| movableclass.cpp:58:21:58:35 | call to MyMovableClass | movableclass.cpp:62:8:62:9 | s1 | |
236252
| movableclass.cpp:59:21:59:30 | call to getTainted | movableclass.cpp:59:21:59:33 | call to MyMovableClass | |
237253
| movableclass.cpp:59:21:59:33 | call to MyMovableClass | movableclass.cpp:63:8:63:9 | s2 | |
238254
| movableclass.cpp:60:18:60:19 | call to MyMovableClass | movableclass.cpp:64:8:64:9 | s3 | |
239255
| movableclass.cpp:64:13:64:18 | call to source | movableclass.cpp:64:13:64:20 | call to MyMovableClass | TAINT |
256+
| movableclass.cpp:64:13:64:20 | call to MyMovableClass | movableclass.cpp:64:8:64:9 | ref arg s3 | TAINT |
257+
| movableclass.cpp:64:13:64:20 | call to MyMovableClass | movableclass.cpp:64:11:64:11 | call to operator= | TAINT |
240258
| stl.cpp:67:12:67:17 | call to source | stl.cpp:71:7:71:7 | a | |
241259
| stl.cpp:68:16:68:20 | 123 | stl.cpp:68:16:68:21 | call to basic_string | TAINT |
242260
| stl.cpp:68:16:68:21 | call to basic_string | stl.cpp:72:7:72:7 | b | |

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

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,7 @@ void test_copyableclass()
4242

4343
sink(s1); // tainted
4444
sink(s2); // tainted
45-
sink(s3); // tainted [NOT DETECTED]
45+
sink(s3); // tainted
4646
}
4747

4848
{
@@ -51,7 +51,7 @@ void test_copyableclass()
5151
s2 = MyMovableClass(source());
5252

5353
sink(s1); // tainted
54-
sink(s2); // tainted [NOT DETECTED]
54+
sink(s2); // tainted
5555
}
5656

5757
{
@@ -61,6 +61,6 @@ void test_copyableclass()
6161

6262
sink(s1);
6363
sink(s2); // tainted
64-
sink(s3 = source()); // tainted [NOT DETECTED]
64+
sink(s3 = source()); // tainted
6565
}
6666
}

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

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,10 @@
11
| copyableclass.cpp:39:8:39:9 | s1 | copyableclass.cpp:33:22:33:27 | call to source |
22
| copyableclass.cpp:40:8:40:9 | s2 | copyableclass.cpp:34:24:34:29 | call to source |
33
| copyableclass.cpp:41:8:41:9 | s3 | copyableclass.cpp:33:22:33:27 | call to source |
4+
| copyableclass.cpp:42:8:42:9 | s4 | copyableclass.cpp:37:8:37:13 | call to source |
45
| copyableclass.cpp:64:8:64:9 | s1 | copyableclass.cpp:59:40:59:45 | call to source |
6+
| copyableclass.cpp:65:8:65:9 | s2 | copyableclass.cpp:62:24:62:29 | call to source |
7+
| copyableclass.cpp:66:11:66:11 | call to operator= | copyableclass.cpp:66:13:66:18 | call to source |
58
| format.cpp:57:8:57:13 | buffer | format.cpp:56:36:56:49 | call to source |
69
| format.cpp:62:8:62:13 | buffer | format.cpp:61:30:61:43 | call to source |
710
| format.cpp:67:8:67:13 | buffer | format.cpp:66:52:66:65 | call to source |
@@ -16,8 +19,11 @@
1619
| format.cpp:158:7:158:27 | ... + ... | format.cpp:148:16:148:30 | call to source |
1720
| movableclass.cpp:43:8:43:9 | s1 | movableclass.cpp:38:21:38:26 | call to source |
1821
| movableclass.cpp:44:8:44:9 | s2 | movableclass.cpp:39:23:39:28 | call to source |
22+
| movableclass.cpp:45:8:45:9 | s3 | movableclass.cpp:41:8:41:13 | call to source |
1923
| movableclass.cpp:53:8:53:9 | s1 | movableclass.cpp:49:38:49:43 | call to source |
24+
| movableclass.cpp:54:8:54:9 | s2 | movableclass.cpp:51:23:51:28 | call to source |
2025
| movableclass.cpp:63:8:63:9 | s2 | movableclass.cpp:22:55:22:60 | call to source |
26+
| movableclass.cpp:64:11:64:11 | call to operator= | movableclass.cpp:64:13:64:18 | call to source |
2127
| stl.cpp:71:7:71:7 | a | stl.cpp:67:12:67:17 | call to source |
2228
| stl.cpp:73:7:73:7 | c | stl.cpp:69:16:69:21 | call to source |
2329
| stl.cpp:75:9:75:13 | call to c_str | stl.cpp:69:16:69:21 | call to source |

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

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,10 @@
11
| copyableclass.cpp:39:8:39:9 | copyableclass.cpp:33:22:33:27 | AST only |
22
| copyableclass.cpp:40:8:40:9 | copyableclass.cpp:34:24:34:29 | AST only |
33
| copyableclass.cpp:41:8:41:9 | copyableclass.cpp:33:22:33:27 | AST only |
4+
| copyableclass.cpp:42:8:42:9 | copyableclass.cpp:37:8:37:13 | AST only |
45
| copyableclass.cpp:64:8:64:9 | copyableclass.cpp:59:40:59:45 | AST only |
6+
| copyableclass.cpp:65:8:65:9 | copyableclass.cpp:62:24:62:29 | AST only |
7+
| copyableclass.cpp:66:11:66:11 | copyableclass.cpp:66:13:66:18 | AST only |
58
| format.cpp:57:8:57:13 | format.cpp:56:36:56:49 | AST only |
69
| format.cpp:62:8:62:13 | format.cpp:61:30:61:43 | AST only |
710
| format.cpp:67:8:67:13 | format.cpp:66:52:66:65 | AST only |
@@ -14,8 +17,11 @@
1417
| format.cpp:110:8:110:14 | format.cpp:109:38:109:52 | AST only |
1518
| movableclass.cpp:43:8:43:9 | movableclass.cpp:38:21:38:26 | AST only |
1619
| movableclass.cpp:44:8:44:9 | movableclass.cpp:39:23:39:28 | AST only |
20+
| movableclass.cpp:45:8:45:9 | movableclass.cpp:41:8:41:13 | AST only |
1721
| movableclass.cpp:53:8:53:9 | movableclass.cpp:49:38:49:43 | AST only |
22+
| movableclass.cpp:54:8:54:9 | movableclass.cpp:51:23:51:28 | AST only |
1823
| movableclass.cpp:63:8:63:9 | movableclass.cpp:22:55:22:60 | AST only |
24+
| movableclass.cpp:64:11:64:11 | movableclass.cpp:64:13:64:18 | AST only |
1925
| stl.cpp:73:7:73:7 | stl.cpp:69:16:69:21 | AST only |
2026
| stl.cpp:75:9:75:13 | stl.cpp:69:16:69:21 | AST only |
2127
| stl.cpp:125:13:125:17 | stl.cpp:117:10:117:15 | AST only |

0 commit comments

Comments
 (0)