Merge pull request #6793 from MathiasVP/add-return-value-deref-to-model-util

C++: Handle return value dereferences in `ModelUtil.qll`

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

@@ -38,5 +38,8 @@ Instruction callOutput(CallInstruction call, FunctionOutput output) {
     effect.getPrimaryInstruction() = call and
     output.isParameterDerefOrQualifierObject(effect.getIndex())
   )
-  // TODO: return value dereference
+  or
+  // TODO: modify this when we get return value dereferences
+  result = call and
+  output.isReturnValueDeref()
 }

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

@@ -64,6 +64,6 @@ void test_copyableclass_declonly()
 
 		sink(s1); // $ ast,ir
 		sink(s2); // $ ast,ir
-		sink(s3 = source()); // $ ast MISSING: ir
+		sink(s3 = source()); // $ ast,ir
 	}
 }

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

@@ -415,10 +415,10 @@ void test_unordered_map()
 	sink(m30["abc"]);
 	sink(m31.try_emplace("abc", source(), 2)); // $ ast,ir
 	sink(m31); // $ ast,ir
-	sink(m31["abc"]); // $ ast MISSING: ir
+	sink(m31["abc"]); // $ ast,ir
 	sink(m32.try_emplace("abc", 1, source())); // $ ast,ir
 	sink(m32); // $ ast,ir
-	sink(m32["abc"]); // $ ast MISSING: ir
+	sink(m32["abc"]); // $ ast,ir
 
 	// additional emplace test cases
 	std::unordered_map<char *, char *> m33;

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

@@ -30,20 +30,20 @@ void test_string()
 	sink(b);
 	sink(c); // $ ast,ir
 	sink(b.c_str());
-	sink(c.c_str()); // $ ast MISSING: ir
+	sink(c.c_str()); // $ ast,ir
 }
 
 void test_strings2()
 {
 	string path1 = user_input();
-	sink(path1.c_str(), "r"); // $ ast MISSING: ir
+	sink(path1.c_str(), "r"); // $ ast,ir
 
 	string path2;
 	path2 = user_input();
-	sink(path2.c_str(), "r"); // $ ast MISSING: ir
+	sink(path2.c_str(), "r"); // $ ast,ir
 
 	string path3(user_input());
-	sink(path3.c_str(), "r"); // $ ast MISSING: ir
+	sink(path3.c_str(), "r"); // $ ast,ir
 }
 
 void test_string3()
@@ -67,7 +67,7 @@ void test_string4()
 	// convert back std::string -> char *
 	cs = ss.c_str();
 
-	sink(cs); // $ ast MISSING: ir
+	sink(cs); // $ ast,ir
 	sink(ss); // $ ast,ir
 }
 
@@ -159,12 +159,12 @@ void test_string_append() {
 		sink(s5); // $ ast,ir
 
 		s6 = s3;
-		sink(s6 += s4); // $ ast MISSING: ir
+		sink(s6 += s4); // $ ast,ir
 		sink(s6); // $ ast,ir
 
 		s7 = s3;
-		sink(s7 += source()); // $ ast MISSING: ir
-		sink(s7 += " "); // $ ast MISSING: ir
+		sink(s7 += source()); // $ ast,ir
+		sink(s7 += " "); // $ ast,ir
 		sink(s7); // $ ast,ir
 
 		s8 = s3;
@@ -196,10 +196,10 @@ void test_string_assign() {
 	sink(s3.assign(s1));
 	sink(s3);
 
-	sink(s4.assign(s2)); // $ ast MISSING: ir
+	sink(s4.assign(s2)); // $ ast,ir
 	sink(s4); // $ ast,ir
 
-	sink(s5.assign(10, c)); // $ ast MISSING: ir
+	sink(s5.assign(10, c)); // $ ast,ir
 	sink(s5); // $ ast,ir
 
 	sink(s6.assign(s1));
@@ -217,15 +217,15 @@ void test_string_insert() {
 	sink(s3);
 
 	s4 = s2;
-	sink(s4.insert(0, s1)); // $ ast MISSING: ir
+	sink(s4.insert(0, s1)); // $ ast,ir
 	sink(s4); // $ ast,ir
 
 	s5 = s1;
-	sink(s5.insert(0, s2)); // $ ast MISSING: ir
+	sink(s5.insert(0, s2)); // $ ast,ir
 	sink(s5); // $ ast,ir
 
 	s6 = s1;
-	sink(s6.insert(0, 10, c)); // $ ast MISSING: ir
+	sink(s6.insert(0, 10, c)); // $ ast,ir
 	sink(s6); // $ ast,ir
 }
 
@@ -240,15 +240,15 @@ void test_string_replace() {
 	sink(s3);
 
 	s4 = s2;
-	sink(s4.replace(1, 2, s1)); // $ ast MISSING: ir
+	sink(s4.replace(1, 2, s1)); // $ ast,ir
 	sink(s4); // $ ast,ir
 
 	s5 = s1;
-	sink(s5.replace(1, 2, s2)); // $ ast MISSING: ir
+	sink(s5.replace(1, 2, s2)); // $ ast,ir
 	sink(s5); // $ ast,ir
 
 	s6 = s1;
-	sink(s6.replace(1, 2, 10, c)); // $ ast MISSING: ir
+	sink(s6.replace(1, 2, 10, c)); // $ ast,ir
 	sink(s6); // $ ast,ir
 }
 
@@ -309,7 +309,7 @@ void test_string_data()
 	std::string b(source());
 
 	sink(a.data());
-	sink(b.data()); // $ ast MISSING: ir
+	sink(b.data()); // $ ast,ir
 	sink(a.length());
 	sink(b.length());
 }
@@ -360,7 +360,7 @@ void test_string_iterators() {
 		std::string s4("world");
 
 		sink(s1);
-		sink(s1.append(s2.begin(), s2.end())); // $ ast MISSING: ir
+		sink(s1.append(s2.begin(), s2.end())); // $ ast,ir
 		sink(s1); // $ ast,ir
 
 		sink(s3);
@@ -433,7 +433,7 @@ void test_string_insert_more()
 	sink(s1.insert(0, cs1));
 	sink(s1);
 
-	sink(s2.insert(0, cs2)); // $ ast MISSING: ir
+	sink(s2.insert(0, cs2)); // $ ast,ir
 	sink(s2); // $ ast,ir
 }
 
@@ -446,7 +446,7 @@ void test_string_iterator_methods()
 		sink(a.insert(a.begin(), 10, 'x'));
 		sink(a);
 
-		sink(b.insert(b.begin(), 10, ns_char::source())); // $ ast MISSING: ir
+		sink(b.insert(b.begin(), 10, ns_char::source())); // $ ast,ir
 		sink(b); // $ ast,ir
 	}
 
@@ -459,10 +459,10 @@ void test_string_iterator_methods()
 		sink(c.insert(c.end(), s1.begin(), s1.end()));
 		sink(c);
 
-		sink(d.insert(d.end(), s2.begin(), s2.end())); // $ ast MISSING: ir
+		sink(d.insert(d.end(), s2.begin(), s2.end())); // $ ast,ir
 		sink(d); // $ ast,ir
 
-		sink(s2.insert(s2.end(), s1.begin(), s1.end())); // $ ast MISSING: ir
+		sink(s2.insert(s2.end(), s1.begin(), s1.end())); // $ ast,ir
 		sink(s2); // $ ast,ir
 	}
 
@@ -475,10 +475,10 @@ void test_string_iterator_methods()
 		sink(e.append(s3.begin(), s3.end()));
 		sink(e);
 
-		sink(f.append(s4.begin(), s4.end())); // $ ast MISSING: ir
+		sink(f.append(s4.begin(), s4.end())); // $ ast,ir
 		sink(f); // $ ast,ir
 
-		sink(s4.append(s3.begin(), s3.end())); // $ ast MISSING: ir
+		sink(s4.append(s3.begin(), s3.end())); // $ ast,ir
 		sink(s4); // $ ast,ir
 	}
 
@@ -491,7 +491,7 @@ void test_string_iterator_methods()
 		sink(g.assign(s5.cbegin(), s5.cend()));
 		sink(g);
 
-		sink(h.assign(s6.cbegin(), s6.cend())); // $ ast MISSING: ir
+		sink(h.assign(s6.cbegin(), s6.cend())); // $ ast,ir
 		sink(h); // $ ast,ir
 
 		sink(s6.assign(s5.cbegin(), s5.cend()));
@@ -519,8 +519,8 @@ void test_string_front_back() {
 	sink(a.front());
 	sink(a.back());
 	a.push_back(ns_char::source());
-	sink(a.front()); // $ SPURIOUS: ast
-	sink(a.back()); // $ ast MISSING: ir
+	sink(a.front()); // $ SPURIOUS: ast,ir
+	sink(a.back()); // $ ast,ir
 }
 
 void test_string_return_assign() {
@@ -533,12 +533,12 @@ void test_string_return_assign() {
 		std::string f("ff");
 
 		sink( a += (b += "bb") );
-		sink( c += (d += source()) ); // $ ast MISSING: ir
-		sink( (e += "ee") += source() ); // $ ast MISSING: ir
-		sink( (f += source()) += "ff" ); // $ ast MISSING: ir
+		sink( c += (d += source()) ); // $ ast,ir
+		sink( (e += "ee") += source() ); // $ ast,ir
+		sink( (f += source()) += "ff" ); // $ ast,ir
 		sink(a);
 		sink(b);
-		sink(c); // $ ast MISSING: ir
+		sink(c); // $ ast,ir
 		sink(d); // $ ast,ir
 		sink(e); // $ ast MISSING: ir
 		sink(f); // $ ast,ir
@@ -553,12 +553,12 @@ void test_string_return_assign() {
 		std::string f("ff");
 
 		sink( a.assign(b.assign("bb")) );
-		sink( c.assign(d.assign(source())) ); // $ ast MISSING: ir
-		sink( e.assign("ee").assign(source()) ); // $ ast MISSING: ir
+		sink( c.assign(d.assign(source())) ); // $ ast,ir
+		sink( e.assign("ee").assign(source()) ); // $ ast,ir
 		sink( f.assign(source()).assign("ff") );
 		sink(a);
 		sink(b);
-		sink(c); // $ ast MISSING: ir
+		sink(c); // $ ast,ir
 		sink(d); // $ ast,ir
 		sink(e); // $ ast MISSING: ir
 		sink(f); // $ SPURIOUS: ast,ir

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

@@ -53,15 +53,15 @@ void test_stringstream_string(int amount)
 	sink(ss7); // $ SPURIOUS: ast,ir
 
 	sink(ss8.put('a'));
-	sink(ss9.put(ns_char::source())); // $ ast MISSING: ir
-	sink(ss10.put('a').put(ns_char::source()).put('z')); // $ ast MISSING: ir
+	sink(ss9.put(ns_char::source())); // $ ast,ir
+	sink(ss10.put('a').put(ns_char::source()).put('z')); // $ ast,ir
 	sink(ss8);
 	sink(ss9); // $ ast,ir
 	sink(ss10); // $ ast MISSING: ir
 
 	sink(ss11.write("begin", 5));
-	sink(ss12.write(source(), 5)); // $ ast MISSING: ir
-	sink(ss13.write("begin", 5).write(source(), amount).write("end", 3)); // $ ast MISSING: ir
+	sink(ss12.write(source(), 5)); // $ ast,ir
+	sink(ss13.write("begin", 5).write(source(), amount).write("end", 3)); // $ ast,ir
 	sink(ss11);
 	sink(ss12); // $ ast,ir
 	sink(ss13); // $ ast MISSING: ir
@@ -73,7 +73,7 @@ void test_stringstream_int(int source)
 	int v1 = 0, v2 = 0;
 
 	sink(ss1 << 1234);
-	sink(ss2 << source); // $ ast MISSING: ir
+	sink(ss2 << source); // $ ast,ir
 	sink(ss1 >> v1);
 	sink(ss2 >> v2); // $ ast,ir
 
@@ -97,7 +97,7 @@ void test_stringstream_constructors()
 	std::stringstream ss6;
 
 	sink(ss5 = std::stringstream("abc"));
-	sink(ss6 = std::stringstream(source())); // $ ast MISSING: ir
+	sink(ss6 = std::stringstream(source())); // $ ast,ir
 
 	sink(ss1);
 	sink(ss2); // $ ast,ir
@@ -193,7 +193,7 @@ void test_stringstream_putback()
 	sink(ss.get());
 	sink(ss.putback('b'));
 	sink(ss.get());
-	sink(ss.putback(ns_char::source())); // $ ast MISSING: ir
+	sink(ss.putback(ns_char::source())); // $ ast,ir
 	sink(ss.get()); // $ ast,ir
 }
 
@@ -263,6 +263,6 @@ void test_chaining()
 	sink(b1); // $ ast,ir
 	sink(b2); // $ ast,ir
 
-	sink(ss2.write("abc", 3).flush().write(source(), 3).flush().write("xyz", 3)); // $ ast MISSING: ir
+	sink(ss2.write("abc", 3).flush().write(source(), 3).flush().write("xyz", 3)); // $ ast,ir
 	sink(ss2); // $ ast MISSING: ir
 }

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

@@ -466,7 +466,7 @@ void test_qualifiers()
 	sink(d.getString());
 	d.setString(strings::source());
 	sink(d); // $ ast,ir
-	sink(d.getString()); // $ ast MISSING: ir
+	sink(d.getString()); // $ ast,ir
 }
 
 // --- non-standard swap ---

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

@@ -68,8 +68,8 @@ void test_element_taint(int x) {
 
 	v5.push_back(source());
 	sink(v5); // $ ast,ir
-	sink(v5.front()); // $ SPURIOUS: ast
-	sink(v5.back()); // $ ast MISSING: ir
+	sink(v5.front()); // $ SPURIOUS: ast,ir
+	sink(v5.back()); // $ ast,ir
 
 	v6.data()[2] = source();
 	sink(v6); // $ ast MISSING: ir
@@ -81,8 +81,8 @@ void test_element_taint(int x) {
 		v7.insert(it, source());
 	}
 	sink(v7); // $ ast,ir
-	sink(v7.front()); // $ ast MISSING: ir
-	sink(v7.back()); // $ SPURIOUS: ast
+	sink(v7.front()); // $ ast,ir
+	sink(v7.back()); // $ SPURIOUS: ast,ir
 
 	{
 		const std::vector<int> &v8c = v8;
@@ -283,8 +283,8 @@ void test_data_more() {
 
 	v1.push_back(source());
 	sink(v1); // $ ast,ir
-	sink(v1.data()); // $ ast MISSING: ir
-	sink(v1.data()[2]); // $ ast MISSING: ir
+	sink(v1.data()); // $ ast,ir
+	sink(v1.data()[2]); // $ ast,ir
 
 	*(v2.data()) = ns_int::source();
 	sink(v2); // $ ast MISSING: ir
@@ -305,10 +305,10 @@ void test_vector_insert() {
 	sink(a.insert(a.end(), b.begin(), b.end()));
 	sink(a);
 
-	sink(c.insert(c.end(), d.begin(), d.end())); // $ ast MISSING: ir
+	sink(c.insert(c.end(), d.begin(), d.end())); // $ ast,ir
 	sink(c); // $ ast,ir
 
-	sink(d.insert(d.end(), a.begin(), a.end())); // $ ast MISSING: ir
+	sink(d.insert(d.end(), a.begin(), a.end())); // $ ast,ir
 	sink(d); // $ ast,ir
 }
 

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

@@ -20,8 +20,17 @@ edges
 | test.cpp:93:17:93:24 | filename indirection | test.cpp:93:11:93:14 | strncat output argument |
 | test.cpp:93:17:93:24 | filename indirection | test.cpp:93:11:93:14 | strncat output argument |
 | test.cpp:106:20:106:25 | call to getenv | test.cpp:107:33:107:36 | path indirection |
+| test.cpp:107:31:107:31 | call to operator+ | test.cpp:108:18:108:22 | call to c_str indirection |
+| test.cpp:107:33:107:36 | path indirection | test.cpp:107:31:107:31 | call to operator+ |
+| test.cpp:107:33:107:36 | path indirection | test.cpp:107:31:107:31 | call to operator+ |
 | test.cpp:113:20:113:25 | call to getenv | test.cpp:114:19:114:22 | path indirection |
+| test.cpp:114:17:114:17 | Call | test.cpp:114:25:114:29 | call to c_str indirection |
+| test.cpp:114:19:114:22 | path indirection | test.cpp:114:17:114:17 | Call |
+| test.cpp:114:19:114:22 | path indirection | test.cpp:114:17:114:17 | Call |
 | test.cpp:119:20:119:25 | call to getenv | test.cpp:120:19:120:22 | path indirection |
+| test.cpp:120:17:120:17 | Call | test.cpp:120:10:120:30 | call to data indirection |
+| test.cpp:120:19:120:22 | path indirection | test.cpp:120:17:120:17 | Call |
+| test.cpp:120:19:120:22 | path indirection | test.cpp:120:17:120:17 | Call |
 | test.cpp:140:9:140:11 | fread output argument | test.cpp:142:31:142:33 | str indirection |
 | test.cpp:142:11:142:17 | sprintf output argument | test.cpp:143:10:143:16 | command indirection |
 | test.cpp:142:31:142:33 | str indirection | test.cpp:142:11:142:17 | sprintf output argument |
@@ -48,10 +57,16 @@ nodes
 | test.cpp:93:17:93:24 | filename indirection | semmle.label | filename indirection |
 | test.cpp:94:45:94:48 | path indirection | semmle.label | path indirection |
 | test.cpp:106:20:106:25 | call to getenv | semmle.label | call to getenv |
+| test.cpp:107:31:107:31 | call to operator+ | semmle.label | call to operator+ |
 | test.cpp:107:33:107:36 | path indirection | semmle.label | path indirection |
+| test.cpp:108:18:108:22 | call to c_str indirection | semmle.label | call to c_str indirection |
 | test.cpp:113:20:113:25 | call to getenv | semmle.label | call to getenv |
+| test.cpp:114:17:114:17 | Call | semmle.label | Call |
 | test.cpp:114:19:114:22 | path indirection | semmle.label | path indirection |
+| test.cpp:114:25:114:29 | call to c_str indirection | semmle.label | call to c_str indirection |
 | test.cpp:119:20:119:25 | call to getenv | semmle.label | call to getenv |
+| test.cpp:120:10:120:30 | call to data indirection | semmle.label | call to data indirection |
+| test.cpp:120:17:120:17 | Call | semmle.label | Call |
 | test.cpp:120:19:120:22 | path indirection | semmle.label | path indirection |
 | test.cpp:140:9:140:11 | fread output argument | semmle.label | fread output argument |
 | test.cpp:142:11:142:17 | sprintf output argument | semmle.label | sprintf output argument |
@@ -64,4 +79,7 @@ subpaths
 | 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 |
 | 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 |
 | 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 |
+| 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+ |
+| 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 |
+| 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 |
 | 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 |