C++: Split StdStringCStr and allow reverse flow on data.

Author: geoffw0

cpp/ql/src/semmle/code/cpp/models/implementations/StdString.qll

@@ -8,10 +8,10 @@ class StdBasicString extends TemplateClass {
 }
 
 /**
- * The `std::string` functions `c_str` and  `data`.
+ * The `std::string` function `c_str`.
  */
 class StdStringCStr extends TaintFunction {
-  StdStringCStr() { this.hasQualifiedName("std", "basic_string", ["c_str", "data"]) }
+  StdStringCStr() { this.hasQualifiedName("std", "basic_string", "c_str") }
 
   override predicate hasTaintFlow(FunctionInput input, FunctionOutput output) {
     // flow from string itself (qualifier) to return value
@@ -20,6 +20,24 @@ class StdStringCStr extends TaintFunction {
   }
 }
 
+/**
+ * The `std::string` function `data`.
+ */
+class StdStringData extends TaintFunction {
+  StdStringData() { this.hasQualifiedName("std", "basic_string", "data") }
+
+  override predicate hasTaintFlow(FunctionInput input, FunctionOutput output) {
+    // flow from string itself (qualifier) to return value
+    input.isQualifierObject() and
+    output.isReturnValue()
+    or
+    // reverse flow from returned reference to the qualifier (for writes to
+    // `data`)
+    input.isReturnValueDeref() and
+    output.isQualifierObject()
+  }
+}
+
 /**
  * The `std::string` function `operator+`.
  */

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

@@ -727,6 +727,7 @@
 | string.cpp:348:2:348:14 | access to array [post update] | string.cpp:348:6:348:9 | call to data [inner post update] |  |
 | string.cpp:348:2:348:34 | ... = ... | string.cpp:348:2:348:14 | access to array [post update] |  |
 | string.cpp:348:6:348:9 | call to data | string.cpp:348:2:348:14 | access to array | TAINT |
+| string.cpp:348:6:348:9 | call to data [inner post update] | string.cpp:348:2:348:4 | ref arg str | TAINT |
 | string.cpp:348:13:348:13 | 1 | string.cpp:348:2:348:14 | access to array | TAINT |
 | string.cpp:348:18:348:32 | call to source | string.cpp:348:2:348:34 | ... = ... |  |
 | string.cpp:350:7:350:9 | str | string.cpp:350:11:350:14 | call to data | TAINT |

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

@@ -346,6 +346,6 @@ void test_string_data_more()
 	std::string str("123");
 
 	str.data()[1] = ns_char::source();
-	sink(str); // tainted [NOT DETECTED]
-	sink(str.data()); // tainted [NOT DETECTED]
+	sink(str); // tainted
+	sink(str.data()); // tainted
 }

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

@@ -94,6 +94,8 @@
 | string.cpp:339:7:339:7 | a | string.cpp:335:9:335:23 | call to source |
 | string.cpp:340:7:340:7 | b | string.cpp:336:12:336:26 | call to source |
 | string.cpp:341:7:341:7 | c | string.cpp:335:9:335:23 | call to source |
+| string.cpp:349:7:349:9 | str | string.cpp:348:18:348:32 | call to source |
+| string.cpp:350:11:350:14 | call to data | string.cpp:348:18:348:32 | call to source |
 | structlikeclass.cpp:35:8:35:9 | s1 | structlikeclass.cpp:29:22:29:27 | call to source |
 | structlikeclass.cpp:36:8:36:9 | s2 | structlikeclass.cpp:30:24:30:29 | call to source |
 | structlikeclass.cpp:37:8:37:9 | s3 | structlikeclass.cpp:29:22:29:27 | call to source |

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

@@ -91,6 +91,8 @@
 | string.cpp:339:7:339:7 | string.cpp:335:9:335:23 | AST only |
 | string.cpp:340:7:340:7 | string.cpp:336:12:336:26 | AST only |
 | string.cpp:341:7:341:7 | string.cpp:335:9:335:23 | AST only |
+| string.cpp:349:7:349:9 | string.cpp:348:18:348:32 | AST only |
+| string.cpp:350:11:350:14 | string.cpp:348:18:348:32 | AST only |
 | structlikeclass.cpp:35:8:35:9 | structlikeclass.cpp:29:22:29:27 | AST only |
 | structlikeclass.cpp:36:8:36:9 | structlikeclass.cpp:30:24:30:29 | AST only |
 | structlikeclass.cpp:37:8:37:9 | structlikeclass.cpp:29:22:29:27 | AST only |