Merge pull request #17209 from RasmusWL/threat-models-stdin

ThreatModels: Add `stdin` kind
github · Aug 16, 2024 · 25fc5f3 · 25fc5f3
2 parents ae013ba + c3d8efc
commit 25fc5f3
Show file tree

Hide file tree

Showing 13 changed files with 50 additions and 21 deletions.
diff --git a/csharp/ql/lib/ext/System.model.yml b/csharp/ql/lib/ext/System.model.yml
@@ -3,9 +3,9 @@ extensions:
       pack: codeql/csharp-all
       extensible: sourceModel
     data:
-      - ["System", "Console", False, "Read", "", "", "ReturnValue", "local", "manual"]
-      - ["System", "Console", False, "ReadKey", "", "", "ReturnValue", "local", "manual"]
-      - ["System", "Console", False, "ReadLine", "", "", "ReturnValue", "local", "manual"]
+      - ["System", "Console", False, "Read", "", "", "ReturnValue", "stdin", "manual"]
+      - ["System", "Console", False, "ReadKey", "", "", "ReturnValue", "stdin", "manual"]
+      - ["System", "Console", False, "ReadLine", "", "", "ReturnValue", "stdin", "manual"]
       - ["System", "Environment", False, "ExpandEnvironmentVariables", "(System.String)", "", "ReturnValue", "environment", "manual"]
       - ["System", "Environment", False, "GetCommandLineArgs", "()", "", "ReturnValue", "commandargs", "manual"]
       - ["System", "Environment", False, "get_CommandLine", "()", "", "ReturnValue", "commandargs", "manual"]

diff --git a/csharp/ql/lib/semmle/code/csharp/security/dataflow/flowsources/Local.qll b/csharp/ql/lib/semmle/code/csharp/security/dataflow/flowsources/Local.qll
@@ -76,3 +76,16 @@ abstract class WindowsRegistrySource extends LocalFlowSource {
 private class ExternalWindowsRegistrySource extends WindowsRegistrySource {
   ExternalWindowsRegistrySource() { sourceNode(this, "windows-registry") }
 }
+
+/**
+ * A dataflow source that represents the reading from stdin.
+ */
+abstract class StdinSource extends LocalFlowSource {
+  override string getThreatModel() { result = "stdin" }
+
+  override string getSourceType() { result = "read from stdin" }
+}
+
+private class ExternalStdinSource extends StdinSource {
+  ExternalStdinSource() { sourceNode(this, "stdin") }
+}
diff --git a/csharp/ql/test/library-tests/dataflow/library/FlowSummaries.expected b/csharp/ql/test/library-tests/dataflow/library/FlowSummaries.expected
@@ -183,10 +183,10 @@ source
 | System.IO;StreamWriter;StreamWriter;(System.String,System.IO.FileStreamOptions);Argument[this];file-write;manual |
 | System.IO;StreamWriter;StreamWriter;(System.String,System.Text.Encoding,System.IO.FileStreamOptions);Argument[this];file-write;manual |
 | System.Net.Sockets;TcpClient;GetStream;();ReturnValue;remote;manual |
-| System;Console;Read;();ReturnValue;local;manual |
-| System;Console;ReadKey;();ReturnValue;local;manual |
-| System;Console;ReadKey;(System.Boolean);ReturnValue;local;manual |
-| System;Console;ReadLine;();ReturnValue;local;manual |
+| System;Console;Read;();ReturnValue;stdin;manual |
+| System;Console;ReadKey;();ReturnValue;stdin;manual |
+| System;Console;ReadKey;(System.Boolean);ReturnValue;stdin;manual |
+| System;Console;ReadLine;();ReturnValue;stdin;manual |
 | System;Environment;ExpandEnvironmentVariables;(System.String);ReturnValue;environment;manual |
 | System;Environment;GetCommandLineArgs;();ReturnValue;commandargs;manual |
 | System;Environment;GetEnvironmentVariable;(System.String);ReturnValue;environment;manual |

diff --git a/csharp/ql/test/query-tests/Security Features/CWE-089/SqlInjection.expected b/csharp/ql/test/query-tests/Security Features/CWE-089/SqlInjection.expected
@@ -7,8 +7,8 @@
 | SqlInjection.cs:83:50:83:55 | access to local variable query1 | SqlInjection.cs:82:21:82:29 | access to property Text : String | SqlInjection.cs:83:50:83:55 | access to local variable query1 | This query depends on $@. | SqlInjection.cs:82:21:82:29 | access to property Text : String | this TextBox text |
 | SqlInjection.cs:93:42:93:52 | access to local variable queryString | SqlInjection.cs:92:21:92:29 | access to property Text : String | SqlInjection.cs:93:42:93:52 | access to local variable queryString | This query depends on $@. | SqlInjection.cs:92:21:92:29 | access to property Text : String | this TextBox text |
 | SqlInjection.cs:94:50:94:52 | access to local variable cmd | SqlInjection.cs:92:21:92:29 | access to property Text : String | SqlInjection.cs:94:50:94:52 | access to local variable cmd | This query depends on $@. | SqlInjection.cs:92:21:92:29 | access to property Text : String | this TextBox text |
-| SqlInjection.cs:104:42:104:52 | access to local variable queryString | SqlInjection.cs:103:21:103:38 | call to method ReadLine : String | SqlInjection.cs:104:42:104:52 | access to local variable queryString | This query depends on $@. | SqlInjection.cs:103:21:103:38 | call to method ReadLine : String | this external |
-| SqlInjection.cs:105:50:105:52 | access to local variable cmd | SqlInjection.cs:103:21:103:38 | call to method ReadLine : String | SqlInjection.cs:105:50:105:52 | access to local variable cmd | This query depends on $@. | SqlInjection.cs:103:21:103:38 | call to method ReadLine : String | this external |
+| SqlInjection.cs:104:42:104:52 | access to local variable queryString | SqlInjection.cs:103:21:103:38 | call to method ReadLine : String | SqlInjection.cs:104:42:104:52 | access to local variable queryString | This query depends on $@. | SqlInjection.cs:103:21:103:38 | call to method ReadLine : String | this read from stdin |
+| SqlInjection.cs:105:50:105:52 | access to local variable cmd | SqlInjection.cs:103:21:103:38 | call to method ReadLine : String | SqlInjection.cs:105:50:105:52 | access to local variable cmd | This query depends on $@. | SqlInjection.cs:103:21:103:38 | call to method ReadLine : String | this read from stdin |
 | SqlInjectionDapper.cs:21:55:21:59 | access to local variable query | SqlInjectionDapper.cs:20:86:20:94 | access to property Text : String | SqlInjectionDapper.cs:21:55:21:59 | access to local variable query | This query depends on $@. | SqlInjectionDapper.cs:20:86:20:94 | access to property Text : String | this TextBox text |
 | SqlInjectionDapper.cs:30:66:30:70 | access to local variable query | SqlInjectionDapper.cs:29:86:29:94 | access to property Text : String | SqlInjectionDapper.cs:30:66:30:70 | access to local variable query | This query depends on $@. | SqlInjectionDapper.cs:29:86:29:94 | access to property Text : String | this TextBox text |
 | SqlInjectionDapper.cs:39:63:39:67 | access to local variable query | SqlInjectionDapper.cs:38:86:38:94 | access to property Text : String | SqlInjectionDapper.cs:39:63:39:67 | access to local variable query | This query depends on $@. | SqlInjectionDapper.cs:38:86:38:94 | access to property Text : String | this TextBox text |
@@ -124,7 +124,7 @@ models
 | 24 | Summary: System.IO; StreamReader; false; StreamReader; (System.IO.Stream,System.Text.Encoding); ; Argument[0]; Argument[this]; taint; manual |
 | 25 | Summary: System.IO; TextReader; true; ReadLine; (); ; Argument[this]; ReturnValue; taint; manual |
 | 26 | Summary: System.Web.UI.WebControls; TextBox; false; get_Text; (); ; Argument[this]; ReturnValue; taint; manual |
-| 27 | Source: System; Console; false; ReadLine; ; ; ReturnValue; local; manual |
+| 27 | Source: System; Console; false; ReadLine; ; ; ReturnValue; stdin; manual |
 | 28 | Summary: System; String; false; Trim; (); ; Argument[this]; ReturnValue; taint; manual |
 nodes
 | SecondOrderSqlInjection.cs:20:31:20:44 | access to local variable customerReader : SqlDataReader | semmle.label | access to local variable customerReader : SqlDataReader |

diff --git a/csharp/ql/test/query-tests/Security Features/CWE-134/UncontrolledFormatString.expected b/csharp/ql/test/query-tests/Security Features/CWE-134/UncontrolledFormatString.expected
@@ -1,5 +1,5 @@
 #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 | thisexternal |
+| 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 |
@@ -17,7 +17,7 @@ edges
 | UncontrolledFormatStringBad.cs:9:25:9:47 | access to property QueryString : NameValueCollection | UncontrolledFormatStringBad.cs:9:25:9:61 | access to indexer : String | provenance | MaD:2 |
 | UncontrolledFormatStringBad.cs:9:25:9:61 | access to indexer : String | UncontrolledFormatStringBad.cs:9:16:9:21 | access to local variable format : String | provenance |  |
 models
-| 1 | Source: System; Console; false; ReadLine; ; ; ReturnValue; local; manual |
+| 1 | Source: System; Console; false; ReadLine; ; ; ReturnValue; stdin; manual |
 | 2 | Summary: System.Collections.Specialized; NameValueCollection; false; get_Item; (System.String); ; Argument[this]; ReturnValue; taint; df-generated |
 nodes
 | ConsoleUncontrolledFormatString.cs:8:13:8:18 | access to local variable format : String | semmle.label | access to local variable format : String |

diff --git a/csharp/ql/test/utils/modelgenerator/dataflow/Sources.cs b/csharp/ql/test/utils/modelgenerator/dataflow/Sources.cs
@@ -14,15 +14,15 @@ public class NewSources
 
 
     // New source
-    // source=Sources;NewSources;false;WrapConsoleReadLine;();;ReturnValue;local;df-generated
+    // source=Sources;NewSources;false;WrapConsoleReadLine;();;ReturnValue;stdin;df-generated
     // neutral=Sources;NewSources;WrapConsoleReadLine;();summary;df-generated
     public string? WrapConsoleReadLine()
     {
         return Console.ReadLine();
     }
 
     // New source
-    // source=Sources;NewSources;false;WrapConsoleReadLineAndProcees;(System.String);;ReturnValue;local;df-generated
+    // source=Sources;NewSources;false;WrapConsoleReadLineAndProcees;(System.String);;ReturnValue;stdin;df-generated
     // neutral=Sources;NewSources;WrapConsoleReadLineAndProcees;(System.String);summary;df-generated
     public string WrapConsoleReadLineAndProcees(string prompt)
     {
@@ -37,7 +37,7 @@ public string WrapConsoleReadLineAndProcees(string prompt)
     }
 
     // New source
-    // source=Sources;NewSources;false;WrapConsoleReadKey;();;ReturnValue;local;df-generated
+    // source=Sources;NewSources;false;WrapConsoleReadKey;();;ReturnValue;stdin;df-generated
     // neutral=Sources;NewSources;WrapConsoleReadKey;();summary;df-generated
     public ConsoleKeyInfo WrapConsoleReadKey()
     {

diff --git a/docs/codeql/reusables/threat-model-description.rst b/docs/codeql/reusables/threat-model-description.rst
@@ -5,7 +5,7 @@ A threat model is a named class of dataflow sources that can be enabled or disab
 The ``kind`` property of the ``sourceModel`` determines which threat model a source is associated with. There are two main categories:
 
 - ``remote`` which represents requests and responses from the network.
-- ``local`` which represents data from local files (``file``), command-line arguments (``commandargs``), database reads (``database``), environment variables(``environment``) and Windows registry values ("windows-registry"). Currently, Windows registry values are used by C# only.
+- ``local`` which represents data from local files (``file``), command-line arguments (``commandargs``), database reads (``database``), environment variables(``environment``), standard input (``stdin``) and Windows registry values ("windows-registry"). Currently, Windows registry values are used by C# only.
 
 Note that subcategories can be turned included or excluded separately, so you can specify ``local`` without ``database``, or just ``commandargs`` and ``environment`` without the rest of ``local``.
 

diff --git a/java/ql/lib/change-notes/2024-08-13-stdin-threat-model.md b/java/ql/lib/change-notes/2024-08-13-stdin-threat-model.md
@@ -0,0 +1,4 @@
+---
+category: minorAnalysis
+---
+* Threat-model for `System.in` changed from `commandargs` to newly created `stdin` (both subgroups of `local`).
diff --git a/java/ql/lib/semmle/code/java/dataflow/FlowSources.qll b/java/ql/lib/semmle/code/java/dataflow/FlowSources.qll
@@ -207,7 +207,8 @@ deprecated class EnvInput extends DataFlow::Node {
   EnvInput() {
     this instanceof EnvironmentInput or
     this instanceof CliInput or
-    this instanceof FileInput
+    this instanceof FileInput or
+    this instanceof StdinInput
   }
 }
 
@@ -234,12 +235,21 @@ private class CliInput extends LocalUserInput {
     exists(Field f | this.asExpr() = f.getAnAccess() |
       f.getAnAnnotation().getType().getQualifiedName() = "org.kohsuke.args4j.Argument"
     )
-    or
+  }
+
+  override string getThreatModel() { result = "commandargs" }
+}
+
+/**
+ * A node with input from stdin.
+ */
+private class StdinInput extends LocalUserInput {
+  StdinInput() {
     // Access to `System.in`.
     exists(Field f | this.asExpr() = f.getAnAccess() | f instanceof SystemIn)
   }
 
-  override string getThreatModel() { result = "commandargs" }
+  override string getThreatModel() { result = "stdin" }
 }
 
 /**

diff --git a/java/ql/test/library-tests/dataflow/threat-models/Test.java b/java/ql/test/library-tests/dataflow/threat-models/Test.java
@@ -59,7 +59,7 @@ public void M4(Statement handle) throws Exception {
   }
 
   public void M5(Statement handle) throws Exception {
-    // Only a source if "commandargs" is a selected threat model.
+    // Only a source if "stdin" is a selected threat model.
     byte[] data = new byte[1024];
     System.in.read(data);
 

diff --git a/java/ql/test/library-tests/dataflow/threat-models/threat-models-flowtest5.ext.yml b/java/ql/test/library-tests/dataflow/threat-models/threat-models-flowtest5.ext.yml
@@ -5,7 +5,7 @@ extensions:
       extensible: threatModelConfiguration
     data:
       - ["environment", true, 0]
-      - ["commandargs", true, 0]
+      - ["stdin", true, 0]
 
   - addsTo:
       pack: codeql/java-all

diff --git a/shared/mad/codeql/mad/ModelValidation.qll b/shared/mad/codeql/mad/ModelValidation.qll
@@ -119,6 +119,7 @@ module KindValidation<KindValidationConfigSig Config> {
         [
           // shared
           "local", "remote", "file", "commandargs", "database", "environment", "reverse-dns",
+          "stdin",
           // Java
           "android-external-storage-dir", "contentprovider",
           // C#

diff --git a/shared/threat-models/ext/threat-model-grouping.model.yml b/shared/threat-models/ext/threat-model-grouping.model.yml
@@ -15,6 +15,7 @@ extensions:
       - ["database", "local"]
       - ["commandargs", "local"]
       - ["environment", "local"]
+      - ["stdin", "local"]
       - ["file", "local"]
       - ["windows-registry", "local"]