Skip to content

Commit ce27acd

Browse files
jbjjbj
authored
Merge pull request #6793 from MathiasVP/add-return-value-deref-to-model-util
C++: Handle return value dereferences in `ModelUtil.qll`
2 parents 62aa7b7 + cc8b581 commit ce27acd

File tree

8 files changed

+76
-55
lines changed

8 files changed

+76
-55
lines changed

‎cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/ModelUtil.qll

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -38,5 +38,8 @@ Instruction callOutput(CallInstruction call, FunctionOutput output) {
3838
effect.getPrimaryInstruction() = call and
3939
output.isParameterDerefOrQualifierObject(effect.getIndex())
4040
)
41-
// TODO: return value dereference
41+
or
42+
// TODO: modify this when we get return value dereferences
43+
result = call and
44+
output.isReturnValueDeref()
4245
}

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -64,6 +64,6 @@ void test_copyableclass_declonly()
6464

6565
sink(s1); // $ ast,ir
6666
sink(s2); // $ ast,ir
67-
sink(s3 = source()); // $ ast MISSING: ir
67+
sink(s3 = source()); // $ ast,ir
6868
}
6969
}

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -415,10 +415,10 @@ void test_unordered_map()
415415
sink(m30["abc"]);
416416
sink(m31.try_emplace("abc", source(), 2)); // $ ast,ir
417417
sink(m31); // $ ast,ir
418-
sink(m31["abc"]); // $ ast MISSING: ir
418+
sink(m31["abc"]); // $ ast,ir
419419
sink(m32.try_emplace("abc", 1, source())); // $ ast,ir
420420
sink(m32); // $ ast,ir
421-
sink(m32["abc"]); // $ ast MISSING: ir
421+
sink(m32["abc"]); // $ ast,ir
422422

423423
// additional emplace test cases
424424
std::unordered_map<char *, char *> m33;

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

Lines changed: 34 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -30,20 +30,20 @@ void test_string()
3030
sink(b);
3131
sink(c); // $ ast,ir
3232
sink(b.c_str());
33-
sink(c.c_str()); // $ ast MISSING: ir
33+
sink(c.c_str()); // $ ast,ir
3434
}
3535

3636
void test_strings2()
3737
{
3838
string path1 = user_input();
39-
sink(path1.c_str(), "r"); // $ ast MISSING: ir
39+
sink(path1.c_str(), "r"); // $ ast,ir
4040

4141
string path2;
4242
path2 = user_input();
43-
sink(path2.c_str(), "r"); // $ ast MISSING: ir
43+
sink(path2.c_str(), "r"); // $ ast,ir
4444

4545
string path3(user_input());
46-
sink(path3.c_str(), "r"); // $ ast MISSING: ir
46+
sink(path3.c_str(), "r"); // $ ast,ir
4747
}
4848

4949
void test_string3()
@@ -67,7 +67,7 @@ void test_string4()
6767
// convert back std::string -> char *
6868
cs = ss.c_str();
6969

70-
sink(cs); // $ ast MISSING: ir
70+
sink(cs); // $ ast,ir
7171
sink(ss); // $ ast,ir
7272
}
7373

@@ -159,12 +159,12 @@ void test_string_append() {
159159
sink(s5); // $ ast,ir
160160

161161
s6 = s3;
162-
sink(s6 += s4); // $ ast MISSING: ir
162+
sink(s6 += s4); // $ ast,ir
163163
sink(s6); // $ ast,ir
164164

165165
s7 = s3;
166-
sink(s7 += source()); // $ ast MISSING: ir
167-
sink(s7 += " "); // $ ast MISSING: ir
166+
sink(s7 += source()); // $ ast,ir
167+
sink(s7 += " "); // $ ast,ir
168168
sink(s7); // $ ast,ir
169169

170170
s8 = s3;
@@ -196,10 +196,10 @@ void test_string_assign() {
196196
sink(s3.assign(s1));
197197
sink(s3);
198198

199-
sink(s4.assign(s2)); // $ ast MISSING: ir
199+
sink(s4.assign(s2)); // $ ast,ir
200200
sink(s4); // $ ast,ir
201201

202-
sink(s5.assign(10, c)); // $ ast MISSING: ir
202+
sink(s5.assign(10, c)); // $ ast,ir
203203
sink(s5); // $ ast,ir
204204

205205
sink(s6.assign(s1));
@@ -217,15 +217,15 @@ void test_string_insert() {
217217
sink(s3);
218218

219219
s4 = s2;
220-
sink(s4.insert(0, s1)); // $ ast MISSING: ir
220+
sink(s4.insert(0, s1)); // $ ast,ir
221221
sink(s4); // $ ast,ir
222222

223223
s5 = s1;
224-
sink(s5.insert(0, s2)); // $ ast MISSING: ir
224+
sink(s5.insert(0, s2)); // $ ast,ir
225225
sink(s5); // $ ast,ir
226226

227227
s6 = s1;
228-
sink(s6.insert(0, 10, c)); // $ ast MISSING: ir
228+
sink(s6.insert(0, 10, c)); // $ ast,ir
229229
sink(s6); // $ ast,ir
230230
}
231231

@@ -240,15 +240,15 @@ void test_string_replace() {
240240
sink(s3);
241241

242242
s4 = s2;
243-
sink(s4.replace(1, 2, s1)); // $ ast MISSING: ir
243+
sink(s4.replace(1, 2, s1)); // $ ast,ir
244244
sink(s4); // $ ast,ir
245245

246246
s5 = s1;
247-
sink(s5.replace(1, 2, s2)); // $ ast MISSING: ir
247+
sink(s5.replace(1, 2, s2)); // $ ast,ir
248248
sink(s5); // $ ast,ir
249249

250250
s6 = s1;
251-
sink(s6.replace(1, 2, 10, c)); // $ ast MISSING: ir
251+
sink(s6.replace(1, 2, 10, c)); // $ ast,ir
252252
sink(s6); // $ ast,ir
253253
}
254254

@@ -309,7 +309,7 @@ void test_string_data()
309309
std::string b(source());
310310

311311
sink(a.data());
312-
sink(b.data()); // $ ast MISSING: ir
312+
sink(b.data()); // $ ast,ir
313313
sink(a.length());
314314
sink(b.length());
315315
}
@@ -360,7 +360,7 @@ void test_string_iterators() {
360360
std::string s4("world");
361361

362362
sink(s1);
363-
sink(s1.append(s2.begin(), s2.end())); // $ ast MISSING: ir
363+
sink(s1.append(s2.begin(), s2.end())); // $ ast,ir
364364
sink(s1); // $ ast,ir
365365

366366
sink(s3);
@@ -433,7 +433,7 @@ void test_string_insert_more()
433433
sink(s1.insert(0, cs1));
434434
sink(s1);
435435

436-
sink(s2.insert(0, cs2)); // $ ast MISSING: ir
436+
sink(s2.insert(0, cs2)); // $ ast,ir
437437
sink(s2); // $ ast,ir
438438
}
439439

@@ -446,7 +446,7 @@ void test_string_iterator_methods()
446446
sink(a.insert(a.begin(), 10, 'x'));
447447
sink(a);
448448

449-
sink(b.insert(b.begin(), 10, ns_char::source())); // $ ast MISSING: ir
449+
sink(b.insert(b.begin(), 10, ns_char::source())); // $ ast,ir
450450
sink(b); // $ ast,ir
451451
}
452452

@@ -459,10 +459,10 @@ void test_string_iterator_methods()
459459
sink(c.insert(c.end(), s1.begin(), s1.end()));
460460
sink(c);
461461

462-
sink(d.insert(d.end(), s2.begin(), s2.end())); // $ ast MISSING: ir
462+
sink(d.insert(d.end(), s2.begin(), s2.end())); // $ ast,ir
463463
sink(d); // $ ast,ir
464464

465-
sink(s2.insert(s2.end(), s1.begin(), s1.end())); // $ ast MISSING: ir
465+
sink(s2.insert(s2.end(), s1.begin(), s1.end())); // $ ast,ir
466466
sink(s2); // $ ast,ir
467467
}
468468

@@ -475,10 +475,10 @@ void test_string_iterator_methods()
475475
sink(e.append(s3.begin(), s3.end()));
476476
sink(e);
477477

478-
sink(f.append(s4.begin(), s4.end())); // $ ast MISSING: ir
478+
sink(f.append(s4.begin(), s4.end())); // $ ast,ir
479479
sink(f); // $ ast,ir
480480

481-
sink(s4.append(s3.begin(), s3.end())); // $ ast MISSING: ir
481+
sink(s4.append(s3.begin(), s3.end())); // $ ast,ir
482482
sink(s4); // $ ast,ir
483483
}
484484

@@ -491,7 +491,7 @@ void test_string_iterator_methods()
491491
sink(g.assign(s5.cbegin(), s5.cend()));
492492
sink(g);
493493

494-
sink(h.assign(s6.cbegin(), s6.cend())); // $ ast MISSING: ir
494+
sink(h.assign(s6.cbegin(), s6.cend())); // $ ast,ir
495495
sink(h); // $ ast,ir
496496

497497
sink(s6.assign(s5.cbegin(), s5.cend()));
@@ -519,8 +519,8 @@ void test_string_front_back() {
519519
sink(a.front());
520520
sink(a.back());
521521
a.push_back(ns_char::source());
522-
sink(a.front()); // $ SPURIOUS: ast
523-
sink(a.back()); // $ ast MISSING: ir
522+
sink(a.front()); // $ SPURIOUS: ast,ir
523+
sink(a.back()); // $ ast,ir
524524
}
525525

526526
void test_string_return_assign() {
@@ -533,12 +533,12 @@ void test_string_return_assign() {
533533
std::string f("ff");
534534

535535
sink( a += (b += "bb") );
536-
sink( c += (d += source()) ); // $ ast MISSING: ir
537-
sink( (e += "ee") += source() ); // $ ast MISSING: ir
538-
sink( (f += source()) += "ff" ); // $ ast MISSING: ir
536+
sink( c += (d += source()) ); // $ ast,ir
537+
sink( (e += "ee") += source() ); // $ ast,ir
538+
sink( (f += source()) += "ff" ); // $ ast,ir
539539
sink(a);
540540
sink(b);
541-
sink(c); // $ ast MISSING: ir
541+
sink(c); // $ ast,ir
542542
sink(d); // $ ast,ir
543543
sink(e); // $ ast MISSING: ir
544544
sink(f); // $ ast,ir
@@ -553,12 +553,12 @@ void test_string_return_assign() {
553553
std::string f("ff");
554554

555555
sink( a.assign(b.assign("bb")) );
556-
sink( c.assign(d.assign(source())) ); // $ ast MISSING: ir
557-
sink( e.assign("ee").assign(source()) ); // $ ast MISSING: ir
556+
sink( c.assign(d.assign(source())) ); // $ ast,ir
557+
sink( e.assign("ee").assign(source()) ); // $ ast,ir
558558
sink( f.assign(source()).assign("ff") );
559559
sink(a);
560560
sink(b);
561-
sink(c); // $ ast MISSING: ir
561+
sink(c); // $ ast,ir
562562
sink(d); // $ ast,ir
563563
sink(e); // $ ast MISSING: ir
564564
sink(f); // $ SPURIOUS: ast,ir

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

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -53,15 +53,15 @@ void test_stringstream_string(int amount)
5353
sink(ss7); // $ SPURIOUS: ast,ir
5454

5555
sink(ss8.put('a'));
56-
sink(ss9.put(ns_char::source())); // $ ast MISSING: ir
57-
sink(ss10.put('a').put(ns_char::source()).put('z')); // $ ast MISSING: ir
56+
sink(ss9.put(ns_char::source())); // $ ast,ir
57+
sink(ss10.put('a').put(ns_char::source()).put('z')); // $ ast,ir
5858
sink(ss8);
5959
sink(ss9); // $ ast,ir
6060
sink(ss10); // $ ast MISSING: ir
6161

6262
sink(ss11.write("begin", 5));
63-
sink(ss12.write(source(), 5)); // $ ast MISSING: ir
64-
sink(ss13.write("begin", 5).write(source(), amount).write("end", 3)); // $ ast MISSING: ir
63+
sink(ss12.write(source(), 5)); // $ ast,ir
64+
sink(ss13.write("begin", 5).write(source(), amount).write("end", 3)); // $ ast,ir
6565
sink(ss11);
6666
sink(ss12); // $ ast,ir
6767
sink(ss13); // $ ast MISSING: ir
@@ -73,7 +73,7 @@ void test_stringstream_int(int source)
7373
int v1 = 0, v2 = 0;
7474

7575
sink(ss1 << 1234);
76-
sink(ss2 << source); // $ ast MISSING: ir
76+
sink(ss2 << source); // $ ast,ir
7777
sink(ss1 >> v1);
7878
sink(ss2 >> v2); // $ ast,ir
7979

@@ -97,7 +97,7 @@ void test_stringstream_constructors()
9797
std::stringstream ss6;
9898

9999
sink(ss5 = std::stringstream("abc"));
100-
sink(ss6 = std::stringstream(source())); // $ ast MISSING: ir
100+
sink(ss6 = std::stringstream(source())); // $ ast,ir
101101

102102
sink(ss1);
103103
sink(ss2); // $ ast,ir
@@ -193,7 +193,7 @@ void test_stringstream_putback()
193193
sink(ss.get());
194194
sink(ss.putback('b'));
195195
sink(ss.get());
196-
sink(ss.putback(ns_char::source())); // $ ast MISSING: ir
196+
sink(ss.putback(ns_char::source())); // $ ast,ir
197197
sink(ss.get()); // $ ast,ir
198198
}
199199

@@ -263,6 +263,6 @@ void test_chaining()
263263
sink(b1); // $ ast,ir
264264
sink(b2); // $ ast,ir
265265

266-
sink(ss2.write("abc", 3).flush().write(source(), 3).flush().write("xyz", 3)); // $ ast MISSING: ir
266+
sink(ss2.write("abc", 3).flush().write(source(), 3).flush().write("xyz", 3)); // $ ast,ir
267267
sink(ss2); // $ ast MISSING: ir
268268
}

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -466,7 +466,7 @@ void test_qualifiers()
466466
sink(d.getString());
467467
d.setString(strings::source());
468468
sink(d); // $ ast,ir
469-
sink(d.getString()); // $ ast MISSING: ir
469+
sink(d.getString()); // $ ast,ir
470470
}
471471

472472
// --- non-standard swap ---

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

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -68,8 +68,8 @@ void test_element_taint(int x) {
6868

6969
v5.push_back(source());
7070
sink(v5); // $ ast,ir
71-
sink(v5.front()); // $ SPURIOUS: ast
72-
sink(v5.back()); // $ ast MISSING: ir
71+
sink(v5.front()); // $ SPURIOUS: ast,ir
72+
sink(v5.back()); // $ ast,ir
7373

7474
v6.data()[2] = source();
7575
sink(v6); // $ ast MISSING: ir
@@ -81,8 +81,8 @@ void test_element_taint(int x) {
8181
v7.insert(it, source());
8282
}
8383
sink(v7); // $ ast,ir
84-
sink(v7.front()); // $ ast MISSING: ir
85-
sink(v7.back()); // $ SPURIOUS: ast
84+
sink(v7.front()); // $ ast,ir
85+
sink(v7.back()); // $ SPURIOUS: ast,ir
8686

8787
{
8888
const std::vector<int> &v8c = v8;
@@ -283,8 +283,8 @@ void test_data_more() {
283283

284284
v1.push_back(source());
285285
sink(v1); // $ ast,ir
286-
sink(v1.data()); // $ ast MISSING: ir
287-
sink(v1.data()[2]); // $ ast MISSING: ir
286+
sink(v1.data()); // $ ast,ir
287+
sink(v1.data()[2]); // $ ast,ir
288288

289289
*(v2.data()) = ns_int::source();
290290
sink(v2); // $ ast MISSING: ir
@@ -305,10 +305,10 @@ void test_vector_insert() {
305305
sink(a.insert(a.end(), b.begin(), b.end()));
306306
sink(a);
307307

308-
sink(c.insert(c.end(), d.begin(), d.end())); // $ ast MISSING: ir
308+
sink(c.insert(c.end(), d.begin(), d.end())); // $ ast,ir
309309
sink(c); // $ ast,ir
310310

311-
sink(d.insert(d.end(), a.begin(), a.end())); // $ ast MISSING: ir
311+
sink(d.insert(d.end(), a.begin(), a.end())); // $ ast,ir
312312
sink(d); // $ ast,ir
313313
}
314314

‎cpp/ql/test/query-tests/Security/CWE/CWE-078/semmle/ExecTainted/ExecTainted.expected

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,8 +20,17 @@ edges
2020
| test.cpp:93:17:93:24 | filename indirection | test.cpp:93:11:93:14 | strncat output argument |
2121
| test.cpp:93:17:93:24 | filename indirection | test.cpp:93:11:93:14 | strncat output argument |
2222
| test.cpp:106:20:106:25 | call to getenv | test.cpp:107:33:107:36 | path indirection |
23+
| test.cpp:107:31:107:31 | call to operator+ | test.cpp:108:18:108:22 | call to c_str indirection |
24+
| test.cpp:107:33:107:36 | path indirection | test.cpp:107:31:107:31 | call to operator+ |
25+
| test.cpp:107:33:107:36 | path indirection | test.cpp:107:31:107:31 | call to operator+ |
2326
| test.cpp:113:20:113:25 | call to getenv | test.cpp:114:19:114:22 | path indirection |
27+
| test.cpp:114:17:114:17 | Call | test.cpp:114:25:114:29 | call to c_str indirection |
28+
| test.cpp:114:19:114:22 | path indirection | test.cpp:114:17:114:17 | Call |
29+
| test.cpp:114:19:114:22 | path indirection | test.cpp:114:17:114:17 | Call |
2430
| test.cpp:119:20:119:25 | call to getenv | test.cpp:120:19:120:22 | path indirection |
31+
| test.cpp:120:17:120:17 | Call | test.cpp:120:10:120:30 | call to data indirection |
32+
| test.cpp:120:19:120:22 | path indirection | test.cpp:120:17:120:17 | Call |
33+
| test.cpp:120:19:120:22 | path indirection | test.cpp:120:17:120:17 | Call |
2534
| test.cpp:140:9:140:11 | fread output argument | test.cpp:142:31:142:33 | str indirection |
2635
| test.cpp:142:11:142:17 | sprintf output argument | test.cpp:143:10:143:16 | command indirection |
2736
| test.cpp:142:31:142:33 | str indirection | test.cpp:142:11:142:17 | sprintf output argument |
@@ -48,10 +57,16 @@ nodes
4857
| test.cpp:93:17:93:24 | filename indirection | semmle.label | filename indirection |
4958
| test.cpp:94:45:94:48 | path indirection | semmle.label | path indirection |
5059
| test.cpp:106:20:106:25 | call to getenv | semmle.label | call to getenv |
60+
| test.cpp:107:31:107:31 | call to operator+ | semmle.label | call to operator+ |
5161
| test.cpp:107:33:107:36 | path indirection | semmle.label | path indirection |
62+
| test.cpp:108:18:108:22 | call to c_str indirection | semmle.label | call to c_str indirection |
5263
| test.cpp:113:20:113:25 | call to getenv | semmle.label | call to getenv |
64+
| test.cpp:114:17:114:17 | Call | semmle.label | Call |
5365
| test.cpp:114:19:114:22 | path indirection | semmle.label | path indirection |
66+
| test.cpp:114:25:114:29 | call to c_str indirection | semmle.label | call to c_str indirection |
5467
| test.cpp:119:20:119:25 | call to getenv | semmle.label | call to getenv |
68+
| test.cpp:120:10:120:30 | call to data indirection | semmle.label | call to data indirection |
69+
| test.cpp:120:17:120:17 | Call | semmle.label | Call |
5570
| test.cpp:120:19:120:22 | path indirection | semmle.label | path indirection |
5671
| test.cpp:140:9:140:11 | fread output argument | semmle.label | fread output argument |
5772
| test.cpp:142:11:142:17 | sprintf output argument | semmle.label | sprintf output argument |
@@ -64,4 +79,7 @@ subpaths
6479
| test.cpp:65:10:65:16 | command | test.cpp:62:9:62:16 | fread output argument | test.cpp:65:10:65:16 | command indirection | This argument to an OS command is derived from $@, dangerously concatenated into $@, and then passed to system(string) | test.cpp:62:9:62:16 | fread output argument | user input (String read by fread) | test.cpp:64:11:64:17 | strncat output argument | strncat output argument |
6580
| test.cpp:85:32:85:38 | command | test.cpp:82:9:82:16 | fread output argument | test.cpp:85:32:85:38 | command indirection | This argument to an OS command is derived from $@, dangerously concatenated into $@, and then passed to execl | test.cpp:82:9:82:16 | fread output argument | user input (String read by fread) | test.cpp:84:11:84:17 | strncat output argument | strncat output argument |
6681
| test.cpp:94:45:94:48 | path | test.cpp:91:9:91:16 | fread output argument | test.cpp:94:45:94:48 | path indirection | This argument to an OS command is derived from $@, dangerously concatenated into $@, and then passed to execl | test.cpp:91:9:91:16 | fread output argument | user input (String read by fread) | test.cpp:93:11:93:14 | strncat output argument | strncat output argument |
82+
| test.cpp:108:18:108:22 | call to c_str | test.cpp:106:20:106:25 | call to getenv | test.cpp:108:18:108:22 | call to c_str indirection | This argument to an OS command is derived from $@, dangerously concatenated into $@, and then passed to system(string) | test.cpp:106:20:106:25 | call to getenv | user input (an environment variable) | test.cpp:107:31:107:31 | call to operator+ | call to operator+ |
83+
| test.cpp:114:25:114:29 | call to c_str | test.cpp:113:20:113:25 | call to getenv | test.cpp:114:25:114:29 | call to c_str indirection | This argument to an OS command is derived from $@, dangerously concatenated into $@, and then passed to system(string) | test.cpp:113:20:113:25 | call to getenv | user input (an environment variable) | test.cpp:114:17:114:17 | Call | Call |
84+
| test.cpp:120:25:120:28 | call to data | test.cpp:119:20:119:25 | call to getenv | test.cpp:120:10:120:30 | call to data indirection | This argument to an OS command is derived from $@, dangerously concatenated into $@, and then passed to system(string) | test.cpp:119:20:119:25 | call to getenv | user input (an environment variable) | test.cpp:120:17:120:17 | Call | Call |
6785
| test.cpp:143:10:143:16 | command | test.cpp:140:9:140:11 | fread output argument | test.cpp:143:10:143:16 | command indirection | This argument to an OS command is derived from $@, dangerously concatenated into $@, and then passed to system(string) | test.cpp:140:9:140:11 | fread output argument | user input (String read by fread) | test.cpp:142:11:142:17 | sprintf output argument | sprintf output argument |

0 commit comments

Comments
 (0)