C++: Upgrade cpp/cleartext-storage-file to full taint flow.

Author: geoffw0

cpp/ql/src/Security/CWE/CWE-311/CleartextFileWrite.ql

@@ -17,6 +17,20 @@ import semmle.code.cpp.security.SensitiveExprs
 import semmle.code.cpp.security.FileWrite
 import semmle.code.cpp.dataflow.DataFlow
 import semmle.code.cpp.valuenumbering.GlobalValueNumbering
+import semmle.code.cpp.dataflow.TaintTracking
+
+/**
+ * Taint flow from a sensitive expression to a `FileWrite` sink.
+ */
+class FromSensitiveConfiguration extends TaintTracking::Configuration {
+  FromSensitiveConfiguration() { this = "FromSensitiveConfiguration" }
+
+  override predicate isSource(DataFlow::Node source) { source.asExpr() instanceof SensitiveExpr }
+
+  override predicate isSink(DataFlow::Node sink) {
+    any(FileWrite w).getASource() = sink.asExpr()
+  }
+}
 
 /**
  * An operation on a filename.
@@ -43,9 +57,13 @@ predicate isFileName(GVN gvn) {
   )
 }
 
-from FileWrite w, SensitiveExpr source, Expr mid, Expr dest
+from FromSensitiveConfiguration config, SensitiveExpr source, Expr mid, FileWrite w, Expr dest
 where
-  DataFlow::localFlow(DataFlow::exprNode(source), DataFlow::exprNode(mid)) and
+  exists(DataFlow::PathNode s, DataFlow::PathNode m |
+    config.hasFlowPath(s, m) and
+    s.getNode().asExpr() = source and
+    m.getNode().asExpr() = mid
+  ) and
   mid = w.getASource() and
   dest = w.getDest() and
   not isFileName(globalValueNumber(source)) and // file names are not passwords

cpp/ql/test/query-tests/Security/CWE/CWE-311/semmle/tests/CleartextFileWrite.expected

@@ -5,6 +5,9 @@
 | test2.cpp:55:2:55:8 | call to fprintf | This write into file 'log' may contain unencrypted data from $@ | test2.cpp:55:40:55:51 | widepassword | this source. |
 | test2.cpp:57:2:57:8 | call to fprintf | This write into file 'log' may contain unencrypted data from $@ | test2.cpp:57:39:57:49 | call to getPassword | this source. |
 | test2.cpp:65:3:65:9 | call to fprintf | This write into file 'log' may contain unencrypted data from $@ | test2.cpp:62:18:62:25 | password | this source. |
+| test2.cpp:73:3:73:9 | call to fprintf | This write into file 'log' may contain unencrypted data from $@ | test2.cpp:72:17:72:24 | password | this source. |
+| test2.cpp:76:3:76:9 | call to fprintf | This write into file 'log' may contain unencrypted data from $@ | test2.cpp:72:17:72:24 | password | this source. |
+| test2.cpp:92:3:92:9 | call to fprintf | This write into file 'log' may contain unencrypted data from $@ | test2.cpp:91:45:91:52 | password | this source. |
 | test.cpp:45:3:45:7 | call to fputs | This write into file 'file' may contain unencrypted data from $@ | test.cpp:45:9:45:19 | thePassword | this source. |
 | test.cpp:70:35:70:35 | call to operator<< | This write into file 'mystream' may contain unencrypted data from $@ | test.cpp:70:38:70:48 | thePassword | this source. |
 | test.cpp:73:37:73:41 | call to write | This write into file 'mystream' may contain unencrypted data from $@ | test.cpp:73:43:73:53 | thePassword | this source. |

cpp/ql/test/query-tests/Security/CWE/CWE-311/semmle/tests/test2.cpp

@@ -70,10 +70,10 @@ void tests(FILE *log, myStruct &s)
 		char buf[1024];
 
 		strcpy(buf, s.password);
-		fprintf(log, "buf = %s\n", buf); // BAD [NOT DETECTED]
+		fprintf(log, "buf = %s\n", buf); // BAD
 
 		strcpy(buf, s.password_hash);
-		fprintf(log, "buf = %s\n", buf); // GOOD
+		fprintf(log, "buf = %s\n", buf); // GOOD [FALSE POSITIVE]
 	}
 
 	fprintf(log, "password = %p\n", s.password); // GOOD
@@ -89,6 +89,6 @@ void tests(FILE *log, myStruct &s)
 		char buffer[1024];
 
 		snprintf(buffer, 1024, "password = %s", s.password);
-		fprintf(log, "log: %s", buffer); // BAD [NOT DETECTED]
+		fprintf(log, "log: %s", buffer); // BAD
 	}
 }