C++: Exclude results formatted with a character other than %s.

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

@@ -25,27 +25,29 @@ predicate filenameOperation(FunctionCall op, Expr path) {
     name =
       [
         "remove", "unlink", "rmdir", "rename", "fopen", "open", "freopen", "_open", "_wopen",
-        "_wfopen", "_fsopen", "_wfsopen", "chmod", "chown", "stat", "lstat", "fstat", "access", "_access", "_waccess", "_access_s", "_waccess_s"
+        "_wfopen", "_fsopen", "_wfsopen", "chmod", "chown", "stat", "lstat", "fstat", "access",
+        "_access", "_waccess", "_access_s", "_waccess_s"
       ] and
-      path = op.getArgument(0)
+    path = op.getArgument(0)
     or
     name = ["fopen_s", "wfopen_s", "rename"] and
     path = op.getArgument(1)
   )
 }
 
-predicate isFileName(GVN gvn)
-{
+predicate isFileName(GVN gvn) {
   exists(FunctionCall op, Expr path |
     filenameOperation(op, path) and
     gvn = globalValueNumber(path)
   )
 }
 
-from FileWrite w, SensitiveExpr source, Expr dest
+from FileWrite w, SensitiveExpr source, Expr mid, Expr dest
 where
-  DataFlow::localFlow(DataFlow::exprNode(source), DataFlow::exprNode(w.getASource())) and
+  DataFlow::localFlow(DataFlow::exprNode(source), DataFlow::exprNode(mid)) and
+  mid = w.getASource() and
   dest = w.getDest() and
-  not isFileName(globalValueNumber(source)) // file names are not passwords
+  not isFileName(globalValueNumber(source)) and // file names are not passwords
+  not exists(string convChar | convChar = w.getSourceConvChar(mid) | not convChar = ["s", "S"]) // ignore things written with other conversion characters
 select w, "This write into file '" + dest.toString() + "' may contain unencrypted data from $@",
   source, "this source."

cpp/ql/src/semmle/code/cpp/security/FileWrite.qll

@@ -19,6 +19,15 @@ class FileWrite extends Expr {
    * Gets the expression for the object being written to.
    */
   Expr getDest() { fileWrite(this, _, result) }
+
+  /**
+   * Gets the conversion character for this write, if it exists and is known. For example in the following code the write of `value1` has conversion character `"s"`, whereas the write of `value2` has no conversion specifier.
+   * ```
+   * fprintf(file, "%s", value1);
+   * stream << value2;
+   * ```
+   */
+  string getSourceConvChar(Expr source) { fileWriteWithConvChar(this, source, result) }
 }
 
 /**
@@ -150,3 +159,22 @@ private predicate fileWrite(Call write, Expr source, Expr dest) {
   // file stream using '<<', 'put' or 'write'
   fileStreamChain(write, source, dest)
 }
+
+/**
+ * Whether the function call is a write to a file from 'source' with
+ * conversion character 'conv'. Does not hold if there isn't a conversion
+ * character, or if it is unknown (for example the format string is not a
+ * constant).
+ */
+private predicate fileWriteWithConvChar(
+  FormattingFunctionCall ffc, Expr source, string conv
+) {
+  // fprintf
+  exists(FormattingFunction f, int n |
+    f = ffc.getTarget() and
+    source = ffc.getFormatArgument(n)
+  |
+    exists(f.getOutputParameterIndex(true)) and
+    conv = ffc.(FormattingFunctionCall).getFormat().(FormatLiteral).getConversionChar(n)
+  )
+}