Skip to content

Commit 25fc5f3

Browse files
RasmusWLRasmusWL
authored
Merge pull request #17209 from RasmusWL/threat-models-stdin
ThreatModels: Add `stdin` kind
2 parents ae013ba + c3d8efc commit 25fc5f3

File tree

13 files changed

+50
-21
lines changed

13 files changed

+50
-21
lines changed

‎csharp/ql/lib/ext/System.model.yml

+3-3
Original file line numberDiff line numberDiff line change
@@ -3,9 +3,9 @@ extensions:
33
pack: codeql/csharp-all
44
extensible: sourceModel
55
data:
6-
- ["System", "Console", False, "Read", "", "", "ReturnValue", "local", "manual"]
7-
- ["System", "Console", False, "ReadKey", "", "", "ReturnValue", "local", "manual"]
8-
- ["System", "Console", False, "ReadLine", "", "", "ReturnValue", "local", "manual"]
6+
- ["System", "Console", False, "Read", "", "", "ReturnValue", "stdin", "manual"]
7+
- ["System", "Console", False, "ReadKey", "", "", "ReturnValue", "stdin", "manual"]
8+
- ["System", "Console", False, "ReadLine", "", "", "ReturnValue", "stdin", "manual"]
99
- ["System", "Environment", False, "ExpandEnvironmentVariables", "(System.String)", "", "ReturnValue", "environment", "manual"]
1010
- ["System", "Environment", False, "GetCommandLineArgs", "()", "", "ReturnValue", "commandargs", "manual"]
1111
- ["System", "Environment", False, "get_CommandLine", "()", "", "ReturnValue", "commandargs", "manual"]

‎csharp/ql/lib/semmle/code/csharp/security/dataflow/flowsources/Local.qll

+13
Original file line numberDiff line numberDiff line change
@@ -76,3 +76,16 @@ abstract class WindowsRegistrySource extends LocalFlowSource {
7676
private class ExternalWindowsRegistrySource extends WindowsRegistrySource {
7777
ExternalWindowsRegistrySource() { sourceNode(this, "windows-registry") }
7878
}
79+
80+
/**
81+
* A dataflow source that represents the reading from stdin.
82+
*/
83+
abstract class StdinSource extends LocalFlowSource {
84+
override string getThreatModel() { result = "stdin" }
85+
86+
override string getSourceType() { result = "read from stdin" }
87+
}
88+
89+
private class ExternalStdinSource extends StdinSource {
90+
ExternalStdinSource() { sourceNode(this, "stdin") }
91+
}

‎csharp/ql/test/library-tests/dataflow/library/FlowSummaries.expected

+4-4
Original file line numberDiff line numberDiff line change
@@ -183,10 +183,10 @@ source
183183
| System.IO;StreamWriter;StreamWriter;(System.String,System.IO.FileStreamOptions);Argument[this];file-write;manual |
184184
| System.IO;StreamWriter;StreamWriter;(System.String,System.Text.Encoding,System.IO.FileStreamOptions);Argument[this];file-write;manual |
185185
| System.Net.Sockets;TcpClient;GetStream;();ReturnValue;remote;manual |
186-
| System;Console;Read;();ReturnValue;local;manual |
187-
| System;Console;ReadKey;();ReturnValue;local;manual |
188-
| System;Console;ReadKey;(System.Boolean);ReturnValue;local;manual |
189-
| System;Console;ReadLine;();ReturnValue;local;manual |
186+
| System;Console;Read;();ReturnValue;stdin;manual |
187+
| System;Console;ReadKey;();ReturnValue;stdin;manual |
188+
| System;Console;ReadKey;(System.Boolean);ReturnValue;stdin;manual |
189+
| System;Console;ReadLine;();ReturnValue;stdin;manual |
190190
| System;Environment;ExpandEnvironmentVariables;(System.String);ReturnValue;environment;manual |
191191
| System;Environment;GetCommandLineArgs;();ReturnValue;commandargs;manual |
192192
| System;Environment;GetEnvironmentVariable;(System.String);ReturnValue;environment;manual |

‎csharp/ql/test/query-tests/Security Features/CWE-089/SqlInjection.expected

+3-3
Original file line numberDiff line numberDiff line change
@@ -7,8 +7,8 @@
77
| 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 |
88
| 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 |
99
| 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 |
10-
| 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 |
11-
| 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 |
10+
| 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 |
11+
| 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 |
1212
| 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 |
1313
| 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 |
1414
| 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
124124
| 24 | Summary: System.IO; StreamReader; false; StreamReader; (System.IO.Stream,System.Text.Encoding); ; Argument[0]; Argument[this]; taint; manual |
125125
| 25 | Summary: System.IO; TextReader; true; ReadLine; (); ; Argument[this]; ReturnValue; taint; manual |
126126
| 26 | Summary: System.Web.UI.WebControls; TextBox; false; get_Text; (); ; Argument[this]; ReturnValue; taint; manual |
127-
| 27 | Source: System; Console; false; ReadLine; ; ; ReturnValue; local; manual |
127+
| 27 | Source: System; Console; false; ReadLine; ; ; ReturnValue; stdin; manual |
128128
| 28 | Summary: System; String; false; Trim; (); ; Argument[this]; ReturnValue; taint; manual |
129129
nodes
130130
| SecondOrderSqlInjection.cs:20:31:20:44 | access to local variable customerReader : SqlDataReader | semmle.label | access to local variable customerReader : SqlDataReader |

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

+2-2
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
#select
2-
| 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 |
2+
| 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 |
33
| 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 |
44
| 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 |
55
| 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
1717
| UncontrolledFormatStringBad.cs:9:25:9:47 | access to property QueryString : NameValueCollection | UncontrolledFormatStringBad.cs:9:25:9:61 | access to indexer : String | provenance | MaD:2 |
1818
| UncontrolledFormatStringBad.cs:9:25:9:61 | access to indexer : String | UncontrolledFormatStringBad.cs:9:16:9:21 | access to local variable format : String | provenance | |
1919
models
20-
| 1 | Source: System; Console; false; ReadLine; ; ; ReturnValue; local; manual |
20+
| 1 | Source: System; Console; false; ReadLine; ; ; ReturnValue; stdin; manual |
2121
| 2 | Summary: System.Collections.Specialized; NameValueCollection; false; get_Item; (System.String); ; Argument[this]; ReturnValue; taint; df-generated |
2222
nodes
2323
| ConsoleUncontrolledFormatString.cs:8:13:8:18 | access to local variable format : String | semmle.label | access to local variable format : String |

‎csharp/ql/test/utils/modelgenerator/dataflow/Sources.cs

+3-3
Original file line numberDiff line numberDiff line change
@@ -14,15 +14,15 @@ public class NewSources
1414

1515

1616
// New source
17-
// source=Sources;NewSources;false;WrapConsoleReadLine;();;ReturnValue;local;df-generated
17+
// source=Sources;NewSources;false;WrapConsoleReadLine;();;ReturnValue;stdin;df-generated
1818
// neutral=Sources;NewSources;WrapConsoleReadLine;();summary;df-generated
1919
public string? WrapConsoleReadLine()
2020
{
2121
return Console.ReadLine();
2222
}
2323

2424
// New source
25-
// source=Sources;NewSources;false;WrapConsoleReadLineAndProcees;(System.String);;ReturnValue;local;df-generated
25+
// source=Sources;NewSources;false;WrapConsoleReadLineAndProcees;(System.String);;ReturnValue;stdin;df-generated
2626
// neutral=Sources;NewSources;WrapConsoleReadLineAndProcees;(System.String);summary;df-generated
2727
public string WrapConsoleReadLineAndProcees(string prompt)
2828
{
@@ -37,7 +37,7 @@ public string WrapConsoleReadLineAndProcees(string prompt)
3737
}
3838

3939
// New source
40-
// source=Sources;NewSources;false;WrapConsoleReadKey;();;ReturnValue;local;df-generated
40+
// source=Sources;NewSources;false;WrapConsoleReadKey;();;ReturnValue;stdin;df-generated
4141
// neutral=Sources;NewSources;WrapConsoleReadKey;();summary;df-generated
4242
public ConsoleKeyInfo WrapConsoleReadKey()
4343
{

‎docs/codeql/reusables/threat-model-description.rst

+1-1
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ A threat model is a named class of dataflow sources that can be enabled or disab
55
The ``kind`` property of the ``sourceModel`` determines which threat model a source is associated with. There are two main categories:
66

77
- ``remote`` which represents requests and responses from the network.
8-
- ``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.
8+
- ``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.
99

1010
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``.
1111

‎java/ql/lib/change-notes/2024-08-13-stdin-threat-model.md

+4
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
---
2+
category: minorAnalysis
3+
---
4+
* Threat-model for `System.in` changed from `commandargs` to newly created `stdin` (both subgroups of `local`).

‎java/ql/lib/semmle/code/java/dataflow/FlowSources.qll

+13-3
Original file line numberDiff line numberDiff line change
@@ -207,7 +207,8 @@ deprecated class EnvInput extends DataFlow::Node {
207207
EnvInput() {
208208
this instanceof EnvironmentInput or
209209
this instanceof CliInput or
210-
this instanceof FileInput
210+
this instanceof FileInput or
211+
this instanceof StdinInput
211212
}
212213
}
213214

@@ -234,12 +235,21 @@ private class CliInput extends LocalUserInput {
234235
exists(Field f | this.asExpr() = f.getAnAccess() |
235236
f.getAnAnnotation().getType().getQualifiedName() = "org.kohsuke.args4j.Argument"
236237
)
237-
or
238+
}
239+
240+
override string getThreatModel() { result = "commandargs" }
241+
}
242+
243+
/**
244+
* A node with input from stdin.
245+
*/
246+
private class StdinInput extends LocalUserInput {
247+
StdinInput() {
238248
// Access to `System.in`.
239249
exists(Field f | this.asExpr() = f.getAnAccess() | f instanceof SystemIn)
240250
}
241251

242-
override string getThreatModel() { result = "commandargs" }
252+
override string getThreatModel() { result = "stdin" }
243253
}
244254

245255
/**

‎java/ql/test/library-tests/dataflow/threat-models/Test.java

+1-1
Original file line numberDiff line numberDiff line change
@@ -59,7 +59,7 @@ public void M4(Statement handle) throws Exception {
5959
}
6060

6161
public void M5(Statement handle) throws Exception {
62-
// Only a source if "commandargs" is a selected threat model.
62+
// Only a source if "stdin" is a selected threat model.
6363
byte[] data = new byte[1024];
6464
System.in.read(data);
6565

‎java/ql/test/library-tests/dataflow/threat-models/threat-models-flowtest5.ext.yml

+1-1
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ extensions:
55
extensible: threatModelConfiguration
66
data:
77
- ["environment", true, 0]
8-
- ["commandargs", true, 0]
8+
- ["stdin", true, 0]
99

1010
- addsTo:
1111
pack: codeql/java-all

‎shared/mad/codeql/mad/ModelValidation.qll

+1
Original file line numberDiff line numberDiff line change
@@ -119,6 +119,7 @@ module KindValidation<KindValidationConfigSig Config> {
119119
[
120120
// shared
121121
"local", "remote", "file", "commandargs", "database", "environment", "reverse-dns",
122+
"stdin",
122123
// Java
123124
"android-external-storage-dir", "contentprovider",
124125
// C#

‎shared/threat-models/ext/threat-model-grouping.model.yml

+1
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ extensions:
1515
- ["database", "local"]
1616
- ["commandargs", "local"]
1717
- ["environment", "local"]
18+
- ["stdin", "local"]
1819
- ["file", "local"]
1920
- ["windows-registry", "local"]
2021

0 commit comments

Comments
 (0)