From b4d35b52c386072c1eb8561cf4e68cc87d292b87 Mon Sep 17 00:00:00 2001 From: Tamas Vajk Date: Tue, 2 Mar 2021 13:22:21 +0100 Subject: [PATCH 1/3] C#: Add Console.Read* to local flow sources --- .../csharp/security/dataflow/flowsources/Local.qll | 14 ++++++++++++++ .../CWE-134/ConsoleUncontrolledFormatString.cs | 13 +++++++++++++ .../CWE-134/UncontrolledFormatString.expected | 4 ++++ 3 files changed, 31 insertions(+) create mode 100644 csharp/ql/test/query-tests/Security Features/CWE-134/ConsoleUncontrolledFormatString.cs diff --git a/csharp/ql/src/semmle/code/csharp/security/dataflow/flowsources/Local.qll b/csharp/ql/src/semmle/code/csharp/security/dataflow/flowsources/Local.qll index e867c9be3ddc..b1b0bf8df850 100644 --- a/csharp/ql/src/semmle/code/csharp/security/dataflow/flowsources/Local.qll +++ b/csharp/ql/src/semmle/code/csharp/security/dataflow/flowsources/Local.qll @@ -20,3 +20,17 @@ class TextFieldSource extends LocalUserInputSource { override string getSourceType() { result = "TextBox text" } } + +/** A call to any `System.Console.Read*` method. */ +class SystemConsoleReadSource extends LocalUserInputSource { + SystemConsoleReadSource() { + this.asExpr() = + any(MethodCall call | + call.getTarget().hasQualifiedName("System.Console", "ReadLine") or + call.getTarget().hasQualifiedName("System.Console", "Read") or + call.getTarget().hasQualifiedName("System.Console", "ReadKey") + ) + } + + override string getSourceType() { result = "TextBox text" } +} diff --git a/csharp/ql/test/query-tests/Security Features/CWE-134/ConsoleUncontrolledFormatString.cs b/csharp/ql/test/query-tests/Security Features/CWE-134/ConsoleUncontrolledFormatString.cs new file mode 100644 index 000000000000..c9d4440cf787 --- /dev/null +++ b/csharp/ql/test/query-tests/Security Features/CWE-134/ConsoleUncontrolledFormatString.cs @@ -0,0 +1,13 @@ +using System; +using System; + +public class Program +{ + public static void Main() + { + var format = Console.ReadLine(); + + // BAD: Uncontrolled format string. + var x = string.Format(format, 1, 2); + } +} 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 index bc87c96b194b..4923cd34e70d 100644 --- 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,8 +1,11 @@ edges +| ConsoleUncontrolledFormatString.cs:8:22:8:39 | call to method ReadLine : String | ConsoleUncontrolledFormatString.cs:11:31:11:36 | access to local variable format | | UncontrolledFormatString.cs:11:23:11:45 | access to property QueryString : NameValueCollection | UncontrolledFormatString.cs:14:23:14:26 | access to local variable path | | UncontrolledFormatString.cs:11:23:11:45 | access to property QueryString : NameValueCollection | UncontrolledFormatString.cs:17:46:17:49 | access to local variable path | | UncontrolledFormatStringBad.cs:9:25:9:47 | access to property QueryString : NameValueCollection | UncontrolledFormatStringBad.cs:12:39:12:44 | access to local variable format | nodes +| 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:11:23:11:45 | access to property QueryString : NameValueCollection | semmle.label | access to property QueryString : NameValueCollection | | UncontrolledFormatString.cs:14:23:14:26 | access to local variable path | semmle.label | access to local variable path | | UncontrolledFormatString.cs:17:46:17:49 | access to local variable path | semmle.label | access to local variable path | @@ -10,6 +13,7 @@ nodes | UncontrolledFormatStringBad.cs:9:25:9:47 | access to property QueryString : NameValueCollection | semmle.label | access to property QueryString : NameValueCollection | | UncontrolledFormatStringBad.cs:12:39:12:44 | access to local variable format | semmle.label | access to local variable format | #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 | $@ flows to here and is used as a format string. | ConsoleUncontrolledFormatString.cs:8:22:8:39 | call to method ReadLine | call to method ReadLine | | UncontrolledFormatString.cs:14:23:14:26 | access to local variable path | UncontrolledFormatString.cs:11:23:11:45 | access to property QueryString : NameValueCollection | UncontrolledFormatString.cs:14:23:14:26 | access to local variable path | $@ flows to here and is used as a format string. | UncontrolledFormatString.cs:11:23:11:45 | access to property QueryString | access to property QueryString | | UncontrolledFormatString.cs:17:46:17:49 | access to local variable path | UncontrolledFormatString.cs:11:23:11:45 | access to property QueryString : NameValueCollection | UncontrolledFormatString.cs:17:46:17:49 | access to local variable path | $@ flows to here and is used as a format string. | UncontrolledFormatString.cs:11:23:11:45 | access to property QueryString | access to property QueryString | | UncontrolledFormatString.cs:34:23:34:31 | access to property Text | UncontrolledFormatString.cs:34:23:34:31 | access to property Text | UncontrolledFormatString.cs:34:23:34:31 | access to property Text | $@ flows to here and is used as a format string. | UncontrolledFormatString.cs:34:23:34:31 | access to property Text | access to property Text | From a8a920c8f02fef08b3200473f502c6c4adac2cd7 Mon Sep 17 00:00:00 2001 From: Tamas Vajk Date: Thu, 22 Apr 2021 11:01:12 +0200 Subject: [PATCH 2/3] Add change note --- csharp/change-notes/2021-04-22-console-read-local-source.md | 2 ++ 1 file changed, 2 insertions(+) create mode 100644 csharp/change-notes/2021-04-22-console-read-local-source.md diff --git a/csharp/change-notes/2021-04-22-console-read-local-source.md b/csharp/change-notes/2021-04-22-console-read-local-source.md new file mode 100644 index 000000000000..9fbb5f2ff32a --- /dev/null +++ b/csharp/change-notes/2021-04-22-console-read-local-source.md @@ -0,0 +1,2 @@ +lgtm,codescanning +* `System.Console.Read` methods have been added as data flow sources of local user input. \ No newline at end of file From 1b4c3c7415598a422e1d9093461529149bcb33a6 Mon Sep 17 00:00:00 2001 From: Tamas Vajk Date: Fri, 23 Apr 2021 13:44:34 +0200 Subject: [PATCH 3/3] Fix code review findings --- .../code/csharp/security/dataflow/flowsources/Local.qll | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/csharp/ql/src/semmle/code/csharp/security/dataflow/flowsources/Local.qll b/csharp/ql/src/semmle/code/csharp/security/dataflow/flowsources/Local.qll index b1b0bf8df850..4c7f70921d90 100644 --- a/csharp/ql/src/semmle/code/csharp/security/dataflow/flowsources/Local.qll +++ b/csharp/ql/src/semmle/code/csharp/security/dataflow/flowsources/Local.qll @@ -26,11 +26,9 @@ class SystemConsoleReadSource extends LocalUserInputSource { SystemConsoleReadSource() { this.asExpr() = any(MethodCall call | - call.getTarget().hasQualifiedName("System.Console", "ReadLine") or - call.getTarget().hasQualifiedName("System.Console", "Read") or - call.getTarget().hasQualifiedName("System.Console", "ReadKey") + call.getTarget().hasQualifiedName("System.Console", ["ReadLine", "Read", "ReadKey"]) ) } - override string getSourceType() { result = "TextBox text" } + override string getSourceType() { result = "System.Console input" } }