Merge pull request github#3365 from hvitved/csharp/format-queries-path-problem

C#: Convert `string.format()` queries to path queries

Author: calumgrant

csharp/ql/src/API Abuse/FormatInvalid.ql

@@ -1,7 +1,7 @@
 /**
  * @name Invalid format string
  * @description Using a format string with an incorrect format causes a 'System.FormatException'.
- * @kind problem
+ * @kind path-problem
  * @problem.severity error
  * @precision high
  * @id cs/invalid-format-string
@@ -11,7 +11,8 @@
 
 import csharp
 import semmle.code.csharp.frameworks.Format
+import FormatFlow
 
-from FormatCall s, InvalidFormatString src
-where src = s.getAFormatSource()
-select src, "Invalid format string used in $@ formatting call.", s, "this"
+from FormatCall s, InvalidFormatString src, PathNode source, PathNode sink
+where hasFlowPath(src, source, s, sink)
+select src, source, sink, "Invalid format string used in $@ formatting call.", s, "this"

csharp/ql/src/API Abuse/FormatMissingArgument.ql

@@ -1,7 +1,7 @@
 /**
  * @name Missing format argument
  * @description Supplying too few arguments to a format string causes a 'System.FormatException'.
- * @kind problem
+ * @kind path-problem
  * @problem.severity error
  * @precision high
  * @id cs/format-argument-missing
@@ -11,11 +11,14 @@
 
 import csharp
 import semmle.code.csharp.frameworks.Format
+import FormatFlow
 
-from FormatCall format, ValidFormatString src, int used, int supplied
+from
+  FormatCall format, ValidFormatString src, int used, int supplied, PathNode source, PathNode sink
 where
-  src = format.getAFormatSource() and
+  hasFlowPath(src, source, format, sink) and
   used = src.getAnInsert() and
   supplied = format.getSuppliedArguments() and
   used >= supplied
-select format, "Argument '{" + used + "}' has not been supplied to $@ format string.", src, "this"
+select format, source, sink, "Argument '{" + used + "}' has not been supplied to $@ format string.",
+  src, "this"

csharp/ql/src/API Abuse/FormatUnusedArgument.ql

@@ -1,7 +1,7 @@
 /**
  * @name Unused format argument
  * @description Supplying more arguments than are required for a format string may indicate an error in the format string.
- * @kind problem
+ * @kind path-problem
  * @problem.severity warning
  * @precision high
  * @id cs/format-argument-unused
@@ -11,11 +11,12 @@
 
 import csharp
 import semmle.code.csharp.frameworks.Format
+import FormatFlow
 
-from FormatCall format, int unused, ValidFormatString src
+from FormatCall format, int unused, ValidFormatString src, PathNode source, PathNode sink
 where
-  src = format.getAFormatSource() and
+  hasFlowPath(src, source, format, sink) and
   unused = format.getAnUnusedArgument(src) and
   not src.getValue() = ""
-select format, "The $@ ignores $@.", src, "format string", format.getSuppliedExpr(unused),
-  "this supplied value"
+select format, source, sink, "The $@ ignores $@.", src, "format string",
+  format.getSuppliedExpr(unused), "this supplied value"

csharp/ql/src/semmle/code/csharp/frameworks/Format.qll

@@ -173,7 +173,7 @@ class InvalidFormatString extends StringLiteral {
 }
 
 /** Provides a dataflow configuration for format strings. */
-private module FormatFlow {
+module FormatFlow {
   private import semmle.code.csharp.dataflow.DataFlow
 
   private class FormatConfiguration extends DataFlow2::Configuration {
@@ -186,12 +186,21 @@ private module FormatFlow {
     }
   }
 
-  predicate hasFlow(StringLiteral lit, Expr format) {
-    exists(DataFlow::Node n1, DataFlow::Node n2, FormatConfiguration conf |
-      n1.asExpr() = lit and n2.asExpr() = format
-    |
-      conf.hasFlow(n1, n2)
-    )
+  query predicate nodes = DataFlow2::PathGraph::nodes/3;
+
+  query predicate edges = DataFlow2::PathGraph::edges/2;
+
+  class PathNode = DataFlow2::PathNode;
+
+  /**
+   * Holds if there is flow from string literal `lit` to the format string in
+   * `call`. `litNode` and `formatNode` are the corresponding data-flow path
+   * nodes.
+   */
+  predicate hasFlowPath(StringLiteral lit, PathNode litNode, FormatCall call, PathNode formatNode) {
+    litNode.getNode().asExpr() = lit and
+    formatNode.getNode().asExpr() = call.getFormatExpr() and
+    any(FormatConfiguration conf).hasFlowPath(litNode, formatNode)
   }
 }
 
@@ -218,10 +227,12 @@ class FormatCall extends MethodCall {
   }
 
   /**
+   * DEPRECATED: Use `FormatFlow::hasFlowPath()` instead.
+   *
    * Gets a format string. Global data flow analysis is applied to retrieve all
    * sources that can reach this method call.
    */
-  StringLiteral getAFormatSource() { FormatFlow::hasFlow(result, this.getFormatExpr()) }
+  deprecated StringLiteral getAFormatSource() { FormatFlow::hasFlowPath(result, _, this, _) }
 
   /**
    * Gets the number of supplied arguments (excluding the format string and format
@@ -245,7 +256,7 @@ class FormatCall extends MethodCall {
   /** Gets a supplied argument that is not used in the format string `src`. */
   int getAnUnusedArgument(ValidFormatString src) {
     result = this.getASuppliedArgument() and
-    src = this.getAFormatSource() and
+    FormatFlow::hasFlowPath(src, _, this, _) and
     not result = src.getAnInsert()
   }
 }