C#: Improve precision of cs/uncontrolled-format-string.

csharp/ql/lib/semmle/code/csharp/commons/Collections.qll

@@ -97,7 +97,8 @@ private class ParamsConstructedCollectionTypes extends ParamsCollectionTypeImpl
       unboundbase instanceof SystemCollectionsGenericIReadOnlyListTInterface or
       unboundbase instanceof SystemSpanStruct or
       unboundbase instanceof SystemReadOnlySpanStruct
-    )
+    ) and
+    not this instanceof SystemStringClass
   }
 
   override Type getElementType() { result = base.getTypeArgument(0) }

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

@@ -8,67 +8,120 @@ private import semmle.code.csharp.frameworks.System
 private import semmle.code.csharp.frameworks.system.Text
 
 /** A method that formats a string, for example `string.Format()`. */
-class FormatMethod extends Method {
-  FormatMethod() {
-    exists(Class declType | declType = this.getDeclaringType() |
+abstract private class FormatMethodImpl extends Method {
+  /**
+   * Gets the argument containing the format string. For example, the argument of
+   * `string.Format(IFormatProvider, String, Object)` is `1`.
+   */
+  abstract int getFormatArgument();
+
+  /**
+   * Gets the argument number of the first supplied insert.
+   */
+  int getFirstArgument() { result = this.getFormatArgument() + 1 }
+}
+
+final class FormatMethod = FormatMethodImpl;
+
+/** A class of types used for formatting. */
+private class FormatType extends Type {
+  FormatType() {
+    this instanceof StringType or
+    this instanceof SystemTextCompositeFormatClass
+  }
+}
+
+private class StringAndStringBuilderFormatMethods extends FormatMethodImpl {
+  StringAndStringBuilderFormatMethods() {
+    (
       this.getParameter(0).getType() instanceof SystemIFormatProviderInterface and
-      this.getParameter(1).getType() instanceof StringType and
+      this.getParameter(1).getType() instanceof FormatType
+      or
+      this.getParameter(0).getType() instanceof StringType
+    ) and
+    (
+      this = any(SystemStringClass c).getFormatMethod()
+      or
+      this = any(SystemTextStringBuilderClass c).getAppendFormatMethod()
+    )
+  }
+
+  override int getFormatArgument() {
+    if this.getParameter(0).getType() instanceof SystemIFormatProviderInterface
+    then result = 1
+    else result = 0
+  }
+}
+
+private class SystemMemoryExtensionsFormatMethods extends FormatMethodImpl {
+  SystemMemoryExtensionsFormatMethods() {
+    this = any(SystemMemoryExtensionsClass c).getTryWriteMethod() and
+    this.getParameter(1).getType() instanceof SystemIFormatProviderInterface and
+    this.getParameter(2).getType() instanceof SystemTextCompositeFormatClass
+  }
+
+  override int getFormatArgument() { result = 2 }
+
+  override int getFirstArgument() { result = this.getFormatArgument() + 2 }
+}
+
+private class SystemConsoleAndSystemIoTextWriterFormatMethods extends FormatMethodImpl {
+  SystemConsoleAndSystemIoTextWriterFormatMethods() {
+    this.getParameter(0).getType() instanceof StringType and
+    this.getNumberOfParameters() > 1 and
+    exists(Class declType | declType = this.getDeclaringType() |
+      this.hasName(["Write", "WriteLine"]) and
       (
-        this = any(SystemStringClass c).getFormatMethod()
+        declType.hasFullyQualifiedName("System", "Console")
         or
-        this = any(SystemTextStringBuilderClass c).getAppendFormatMethod()
+        declType.hasFullyQualifiedName("System.IO", "TextWriter")
       )
-      or
-      this.getParameter(0).getType() instanceof StringType and
+    )
+  }
+
+  override int getFormatArgument() { result = 0 }
+}
+
+private class SystemDiagnosticsDebugAssert extends FormatMethodImpl {
+  SystemDiagnosticsDebugAssert() {
+    this.hasName("Assert") and
+    this.getDeclaringType().hasFullyQualifiedName("System.Diagnostics", "Debug") and
+    this.getNumberOfParameters() = 4
+  }
+
+  override int getFormatArgument() { result = 2 }
+}
+
+private class SystemDiagnosticsFormatMethods extends FormatMethodImpl {
+  SystemDiagnosticsFormatMethods() {
+    this.getParameter(0).getType() instanceof StringType and
+    this.getNumberOfParameters() > 1 and
+    exists(Class declType |
+      declType = this.getDeclaringType() and
+      declType.getNamespace().getFullName() = "System.Diagnostics"
+    |
+      declType.hasName("Trace") and
       (
-        this = any(SystemStringClass c).getFormatMethod()
-        or
-        this = any(SystemTextStringBuilderClass c).getAppendFormatMethod()
-        or
-        (this.hasName("Write") or this.hasName("WriteLine")) and
-        (
-          declType.hasFullyQualifiedName("System", "Console")
-          or
-          declType.hasFullyQualifiedName("System.IO", "TextWriter")
-          or
-          declType.hasFullyQualifiedName("System.Diagnostics", "Debug") and
-          this.getParameter(1).getType() instanceof ArrayType
-        )
-        or
-        declType.hasFullyQualifiedName("System.Diagnostics", "Trace") and
-        (
-          this.hasName("TraceError") or
-          this.hasName("TraceInformation") or
-          this.hasName("TraceWarning")
-        )
+        this.hasName("TraceError")
         or
-        this.hasName("TraceInformation") and
-        declType.hasFullyQualifiedName("System.Diagnostics", "TraceSource")
+        this.hasName("TraceInformation")
         or
-        this.hasName("Print") and
-        declType.hasFullyQualifiedName("System.Diagnostics", "Debug")
+        this.hasName("TraceWarning")
       )
       or
-      this.hasName("Assert") and
-      declType.hasFullyQualifiedName("System.Diagnostics", "Debug") and
-      this.getNumberOfParameters() = 4
+      declType.hasName("TraceSource") and this.hasName("TraceInformation")
+      or
+      declType.hasName("Debug") and
+      (
+        this.hasName("Print")
+        or
+        this.hasName(["Write", "WriteLine"]) and
+        this.getParameter(1).getType() instanceof ArrayType
+      )
     )
   }
 
-  /**
-   * Gets the argument containing the format string. For example, the argument of
-   * `string.Format(IFormatProvider, String, Object)` is `1`.
-   */
-  int getFormatArgument() {
-    if this.getParameter(0).getType() instanceof SystemIFormatProviderInterface
-    then result = 1
-    else
-      if
-        this.hasName("Assert") and
-        this.getDeclaringType().hasFullyQualifiedName("System.Diagnostics", "Debug")
-      then result = 2
-      else result = 0
-  }
+  override int getFormatArgument() { result = 0 }
 }
 
 pragma[nomagic]
@@ -194,24 +247,36 @@ class FormatCall extends MethodCall {
   int getFormatArgument() { result = this.getTarget().(FormatMethod).getFormatArgument() }
 
   /** Gets the argument number of the first supplied insert. */
-  int getFirstArgument() { result = this.getFormatArgument() + 1 }
+  int getFirstArgument() { result = this.getTarget().(FormatMethod).getFirstArgument() }
 
   /** Holds if this call has one or more insertions. */
   predicate hasInsertions() { exists(this.getArgument(this.getFirstArgument())) }
 
-  /** Holds if the arguments are supplied in an array, not individually. */
-  predicate hasArrayExpr() {
+  /**
+   * DEPRECATED: use `hasCollectionExpr` instead.
+   *
+   * Holds if the arguments are supplied in an array, not individually.
+   */
+  deprecated predicate hasArrayExpr() {
     this.getNumberOfArguments() = this.getFirstArgument() + 1 and
     this.getArgument(this.getFirstArgument()).getType() instanceof ArrayType
   }
 
+  /**
+   * Holds if the arguments are supplied in a collection, not individually.
+   */
+  predicate hasCollectionExpr() {
+    this.getNumberOfArguments() = this.getFirstArgument() + 1 and
+    this.getArgument(this.getFirstArgument()).getType() instanceof ParamsCollectionType
+  }
+
   /**
    * Gets the number of supplied arguments (excluding the format string and format
    * provider). Does not return a value if the arguments are supplied in an array,
    * in which case we generally can't assess the size of the array.
    */
   int getSuppliedArguments() {
-    not this.hasArrayExpr() and
+    not this.hasCollectionExpr() and
     result = this.getNumberOfArguments() - this.getFirstArgument()
   }
 
@@ -224,3 +289,31 @@ class FormatCall extends MethodCall {
     result = this.getArgument(this.getFirstArgument() + index)
   }
 }
+
+/**
+ * A method call to a method that parses a format string, for example a call
+ * to `string.Format()`.
+ */
+abstract private class FormatStringParseCallImpl extends MethodCall {
+  /**
+   * Gets the expression used as the format string.
+   */
+  abstract Expr getFormatExpr();
+}
+
+final class FormatStringParseCall = FormatStringParseCallImpl;
+
+private class OrdinaryFormatCall extends FormatStringParseCallImpl instanceof FormatCall {
+  override Expr getFormatExpr() { result = FormatCall.super.getFormatExpr() }
+}
+
+/**
+ * A method call to `System.Text.CompositeFormat.Parse`.
+ */
+class ParseFormatStringCall extends FormatStringParseCallImpl {
+  ParseFormatStringCall() {
+    this.getTarget() = any(SystemTextCompositeFormatClass x).getParseMethod()
+  }
+
+  override Expr getFormatExpr() { result = this.getArgument(0) }
+}

csharp/ql/lib/semmle/code/csharp/frameworks/System.qll

@@ -365,7 +365,7 @@ class SystemStringClass extends StringType {
   /** Gets a `Format(...)` method. */
   Method getFormatMethod() {
     result.getDeclaringType() = this and
-    result.hasName("Format") and
+    result.getName().regexpMatch("Format(<.*>)?") and
     result.getNumberOfParameters() in [2 .. 5] and
     result.getReturnType() instanceof StringType
   }
@@ -751,6 +751,18 @@ class SystemNotImplementedExceptionClass extends SystemClass {
   SystemNotImplementedExceptionClass() { this.hasName("NotImplementedException") }
 }
 
+/** The `System.MemoryExtensions` class. */
+class SystemMemoryExtensionsClass extends SystemClass {
+  SystemMemoryExtensionsClass() { this.hasName("MemoryExtensions") }
+
+  /** Gets a `TryWrite` method. */
+  Method getTryWriteMethod() {
+    result.getDeclaringType() = this and
+    result.getName().regexpMatch("TryWrite(<.*>)?") and
+    result.getParameter(0).getType().getUnboundDeclaration() instanceof SystemSpanStruct
+  }
+}
+
 /** The `System.DateTime` struct. */
 class SystemDateTimeStruct extends SystemStruct {
   SystemDateTimeStruct() { this.hasName("DateTime") }

csharp/ql/lib/semmle/code/csharp/frameworks/system/Text.qll

@@ -22,7 +22,12 @@ class SystemTextStringBuilderClass extends SystemTextClass {
   SystemTextStringBuilderClass() { this.hasName("StringBuilder") }
 
   /** Gets the `AppendFormat` method. */
-  Method getAppendFormatMethod() { result = this.getAMethod("AppendFormat") }
+  Method getAppendFormatMethod() {
+    exists(string name |
+      name.regexpMatch("AppendFormat(<.*>)?") and
+      result = this.getAMethod(name)
+    )
+  }
 }
 
 /** The `System.Text.Encoding` class. */
@@ -38,3 +43,11 @@ class SystemTextEncodingClass extends SystemTextClass {
   /** Gets the `GetChars` method. */
   Method getGetCharsMethod() { result = this.getAMethod("GetChars") }
 }
+
+/** The `System.Text.CompositeFormat` class */
+class SystemTextCompositeFormatClass extends SystemTextClass {
+  SystemTextCompositeFormatClass() { this.hasName("CompositeFormat") }
+
+  /** Gets the `Parse` method. */
+  Method getParseMethod() { result = this.getAMethod("Parse") }
+}