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

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

@@ -289,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/src/API Abuse/FormatInvalid.ql

@@ -15,22 +15,6 @@ import semmle.code.csharp.frameworks.system.Text
 import semmle.code.csharp.frameworks.Format
 import FormatFlow::PathGraph
 
-abstract class FormatStringParseCall extends MethodCall {
-  abstract Expr getFormatExpr();
-}
-
-class OrdinaryFormatCall extends FormatStringParseCall instanceof FormatCall {
-  override Expr getFormatExpr() { result = FormatCall.super.getFormatExpr() }
-}
-
-class ParseFormatStringCall extends FormatStringParseCall {
-  ParseFormatStringCall() {
-    this.getTarget() = any(SystemTextCompositeFormatClass x).getParseMethod()
-  }
-
-  override Expr getFormatExpr() { result = this.getArgument(0) }
-}
-
 module FormatInvalidConfig implements DataFlow::ConfigSig {
   predicate isSource(DataFlow::Node n) { n.asExpr() instanceof StringLiteral }
 

csharp/ql/src/Security Features/CWE-134/UncontrolledFormatString.ql

@@ -20,7 +20,7 @@ module FormatStringConfig implements DataFlow::ConfigSig {
   predicate isSource(DataFlow::Node source) { source instanceof ActiveThreatModelSource }
 
   predicate isSink(DataFlow::Node sink) {
-    sink.asExpr() = any(FormatCall call | call.hasInsertions()).getFormatExpr()
+    sink.asExpr() = any(FormatStringParseCall call).getFormatExpr()
   }
 }
 

csharp/ql/src/change-notes/2025-04-10-uncontrolled-format-string.md

@@ -0,0 +1,4 @@
+---
+category: minorAnalysis
+---
+* The precision of the query `cs/uncontrolled-format-string` has been improved. Calls to `System.Text.CompositeFormat.Parse` are now considered a format like method call.

csharp/ql/test/query-tests/Security Features/CWE-134/ConsoleUncontrolledFormatString.cs

@@ -5,9 +5,9 @@ public class Program
 {
     public static void Main()
     {
-        var format = Console.ReadLine();
+        var format = Console.ReadLine(); // $ Source
 
         // BAD: Uncontrolled format string.
-        var x = string.Format(format, 1, 2);
+        var x = string.Format(format, 1, 2); // $ Alert
     }
 }

csharp/ql/test/query-tests/Security Features/CWE-134/UncontrolledFormatString.cs

@@ -1,18 +1,19 @@
 using System;
+using System.Text;
 using System.IO;
 using System.Web;
 
 public class TaintedPathHandler : IHttpHandler
 {
     public void ProcessRequest(HttpContext ctx)
     {
-        String path = ctx.Request.QueryString["page"];
+        String path = ctx.Request.QueryString["page"]; // $ Source
 
         // BAD: Uncontrolled format string.
-        String.Format(path, "Do not do this");
+        String.Format(path, "Do not do this"); // $ Alert
 
         // BAD: Using an IFormatProvider.
-        String.Format((IFormatProvider)null, path, "Do not do this");
+        String.Format((IFormatProvider)null, path, "Do not do this"); // $ Alert
 
         // GOOD: Not the format string.
         String.Format("Do not do this", path);
@@ -22,13 +23,16 @@ public void ProcessRequest(HttpContext ctx)
 
         // GOOD: Not a formatting call
         Console.WriteLine(path);
+
+        // BAD: Uncontrolled format string.
+        CompositeFormat.Parse(path); // $ Alert
     }
 
     System.Windows.Forms.TextBox box1;
 
     void OnButtonClicked()
     {
         // BAD: Uncontrolled format string.
-        String.Format(box1.Text, "Do not do this");
+        String.Format(box1.Text, "Do not do this"); // $ Alert
     }
 }

csharp/ql/test/query-tests/Security Features/CWE-134/UncontrolledFormatString.expected

@@ -1,17 +1,19 @@
 #select
 | ConsoleUncontrolledFormatString.cs:11:31:11:36 | access to local variable format | ConsoleUncontrolledFormatString.cs:8:22:8:39 | call to method ReadLine : String | ConsoleUncontrolledFormatString.cs:11:31:11:36 | access to local variable format | This format string depends on $@. | ConsoleUncontrolledFormatString.cs:8:22:8:39 | call to method ReadLine | thisread from stdin |
-| UncontrolledFormatString.cs:12:23:12:26 | access to local variable path | UncontrolledFormatString.cs:9:23:9:45 | access to property QueryString : NameValueCollection | UncontrolledFormatString.cs:12:23:12:26 | access to local variable path | This format string depends on $@. | UncontrolledFormatString.cs:9:23:9:45 | access to property QueryString | thisASP.NET query string |
-| UncontrolledFormatString.cs:15:46:15:49 | access to local variable path | UncontrolledFormatString.cs:9:23:9:45 | access to property QueryString : NameValueCollection | UncontrolledFormatString.cs:15:46:15:49 | access to local variable path | This format string depends on $@. | UncontrolledFormatString.cs:9:23:9:45 | access to property QueryString | thisASP.NET query string |
-| UncontrolledFormatString.cs:32:23:32:31 | access to property Text | UncontrolledFormatString.cs:32:23:32:31 | access to property Text | UncontrolledFormatString.cs:32:23:32:31 | access to property Text | This format string depends on $@. | UncontrolledFormatString.cs:32:23:32:31 | access to property Text | thisTextBox text |
+| UncontrolledFormatString.cs:13:23:13:26 | access to local variable path | UncontrolledFormatString.cs:10:23:10:45 | access to property QueryString : NameValueCollection | UncontrolledFormatString.cs:13:23:13:26 | access to local variable path | This format string depends on $@. | UncontrolledFormatString.cs:10:23:10:45 | access to property QueryString | thisASP.NET query string |
+| UncontrolledFormatString.cs:16:46:16:49 | access to local variable path | UncontrolledFormatString.cs:10:23:10:45 | access to property QueryString : NameValueCollection | UncontrolledFormatString.cs:16:46:16:49 | access to local variable path | This format string depends on $@. | UncontrolledFormatString.cs:10:23:10:45 | access to property QueryString | thisASP.NET query string |
+| UncontrolledFormatString.cs:28:31:28:34 | access to local variable path | UncontrolledFormatString.cs:10:23:10:45 | access to property QueryString : NameValueCollection | UncontrolledFormatString.cs:28:31:28:34 | access to local variable path | This format string depends on $@. | UncontrolledFormatString.cs:10:23:10:45 | access to property QueryString | thisASP.NET query string |
+| UncontrolledFormatString.cs:36:23:36:31 | access to property Text | UncontrolledFormatString.cs:36:23:36:31 | access to property Text | UncontrolledFormatString.cs:36:23:36:31 | access to property Text | This format string depends on $@. | UncontrolledFormatString.cs:36:23:36:31 | access to property Text | thisTextBox text |
 | UncontrolledFormatStringBad.cs:12:39:12:44 | access to local variable format | UncontrolledFormatStringBad.cs:9:25:9:47 | access to property QueryString : NameValueCollection | UncontrolledFormatStringBad.cs:12:39:12:44 | access to local variable format | This format string depends on $@. | UncontrolledFormatStringBad.cs:9:25:9:47 | access to property QueryString | thisASP.NET query string |
 edges
 | ConsoleUncontrolledFormatString.cs:8:13:8:18 | access to local variable format : String | ConsoleUncontrolledFormatString.cs:11:31:11:36 | access to local variable format | provenance |  |
 | ConsoleUncontrolledFormatString.cs:8:22:8:39 | call to method ReadLine : String | ConsoleUncontrolledFormatString.cs:8:13:8:18 | access to local variable format : String | provenance | Src:MaD:1  |
-| UncontrolledFormatString.cs:9:16:9:19 | access to local variable path : String | UncontrolledFormatString.cs:12:23:12:26 | access to local variable path | provenance |  |
-| UncontrolledFormatString.cs:9:16:9:19 | access to local variable path : String | UncontrolledFormatString.cs:15:46:15:49 | access to local variable path | provenance |  |
-| UncontrolledFormatString.cs:9:23:9:45 | access to property QueryString : NameValueCollection | UncontrolledFormatString.cs:9:16:9:19 | access to local variable path : String | provenance |  |
-| UncontrolledFormatString.cs:9:23:9:45 | access to property QueryString : NameValueCollection | UncontrolledFormatString.cs:9:23:9:53 | access to indexer : String | provenance | MaD:2 |
-| UncontrolledFormatString.cs:9:23:9:53 | access to indexer : String | UncontrolledFormatString.cs:9:16:9:19 | access to local variable path : String | provenance |  |
+| UncontrolledFormatString.cs:10:16:10:19 | access to local variable path : String | UncontrolledFormatString.cs:13:23:13:26 | access to local variable path | provenance |  |
+| UncontrolledFormatString.cs:10:16:10:19 | access to local variable path : String | UncontrolledFormatString.cs:16:46:16:49 | access to local variable path | provenance |  |
+| UncontrolledFormatString.cs:10:16:10:19 | access to local variable path : String | UncontrolledFormatString.cs:28:31:28:34 | access to local variable path | provenance |  |
+| UncontrolledFormatString.cs:10:23:10:45 | access to property QueryString : NameValueCollection | UncontrolledFormatString.cs:10:16:10:19 | access to local variable path : String | provenance |  |
+| UncontrolledFormatString.cs:10:23:10:45 | access to property QueryString : NameValueCollection | UncontrolledFormatString.cs:10:23:10:53 | access to indexer : String | provenance | MaD:2 |
+| UncontrolledFormatString.cs:10:23:10:53 | access to indexer : String | UncontrolledFormatString.cs:10:16:10:19 | access to local variable path : String | provenance |  |
 | UncontrolledFormatStringBad.cs:9:16:9:21 | access to local variable format : String | UncontrolledFormatStringBad.cs:12:39:12:44 | access to local variable format | provenance |  |
 | UncontrolledFormatStringBad.cs:9:25:9:47 | access to property QueryString : NameValueCollection | UncontrolledFormatStringBad.cs:9:16:9:21 | access to local variable format : String | provenance |  |
 | UncontrolledFormatStringBad.cs:9:25:9:47 | access to property QueryString : NameValueCollection | UncontrolledFormatStringBad.cs:9:25:9:61 | access to indexer : String | provenance | MaD:2 |
@@ -23,12 +25,13 @@ nodes
 | ConsoleUncontrolledFormatString.cs:8:13:8:18 | access to local variable format : String | semmle.label | access to local variable format : String |
 | ConsoleUncontrolledFormatString.cs:8:22:8:39 | call to method ReadLine : String | semmle.label | call to method ReadLine : String |
 | ConsoleUncontrolledFormatString.cs:11:31:11:36 | access to local variable format | semmle.label | access to local variable format |
-| UncontrolledFormatString.cs:9:16:9:19 | access to local variable path : String | semmle.label | access to local variable path : String |
-| UncontrolledFormatString.cs:9:23:9:45 | access to property QueryString : NameValueCollection | semmle.label | access to property QueryString : NameValueCollection |
-| UncontrolledFormatString.cs:9:23:9:53 | access to indexer : String | semmle.label | access to indexer : String |
-| UncontrolledFormatString.cs:12:23:12:26 | access to local variable path | semmle.label | access to local variable path |
-| UncontrolledFormatString.cs:15:46:15:49 | access to local variable path | semmle.label | access to local variable path |
-| UncontrolledFormatString.cs:32:23:32:31 | access to property Text | semmle.label | access to property Text |
+| UncontrolledFormatString.cs:10:16:10:19 | access to local variable path : String | semmle.label | access to local variable path : String |
+| UncontrolledFormatString.cs:10:23:10:45 | access to property QueryString : NameValueCollection | semmle.label | access to property QueryString : NameValueCollection |
+| UncontrolledFormatString.cs:10:23:10:53 | access to indexer : String | semmle.label | access to indexer : String |
+| UncontrolledFormatString.cs:13:23:13:26 | access to local variable path | semmle.label | access to local variable path |
+| UncontrolledFormatString.cs:16:46:16:49 | access to local variable path | semmle.label | access to local variable path |
+| UncontrolledFormatString.cs:28:31:28:34 | access to local variable path | semmle.label | access to local variable path |
+| UncontrolledFormatString.cs:36:23:36:31 | access to property Text | semmle.label | access to property Text |
 | UncontrolledFormatStringBad.cs:9:16:9:21 | access to local variable format : String | semmle.label | access to local variable format : String |
 | UncontrolledFormatStringBad.cs:9:25:9:47 | access to property QueryString : NameValueCollection | semmle.label | access to property QueryString : NameValueCollection |
 | UncontrolledFormatStringBad.cs:9:25:9:61 | access to indexer : String | semmle.label | access to indexer : String |

csharp/ql/test/query-tests/Security Features/CWE-134/UncontrolledFormatString.qlref

@@ -1,2 +1,4 @@
 query: Security Features/CWE-134/UncontrolledFormatString.ql
-postprocess: utils/test/PrettyPrintModels.ql
+postprocess:
+  - utils/test/PrettyPrintModels.ql
+  - utils/test/InlineExpectationsTestQuery.ql

csharp/ql/test/query-tests/Security Features/CWE-134/UncontrolledFormatStringBad.cs

@@ -6,9 +6,9 @@ public class HttpHandler : IHttpHandler
 
     public void ProcessRequest(HttpContext ctx)
     {
-        string format = ctx.Request.QueryString["nameformat"];
+        string format = ctx.Request.QueryString["nameformat"]; // $ Source
 
         // BAD: Uncontrolled format string.
-        FormattedName = string.Format(format, Surname, Forenames);
+        FormattedName = string.Format(format, Surname, Forenames); // $ Alert
     }
 }