Merge pull request github#6556 from hvitved/csharp/insecure-sql-conn-flow

hvitved · web-flow · commit 05b45da42fc2 · 2021-08-30T11:31:22.000+02:00
C#: Use data flow instead of taint tracking in `InsecureSQLConnection.ql`
diff --git a/csharp/ql/src/Security Features/CWE-327/InsecureSQLConnection.ql b/csharp/ql/src/Security Features/CWE-327/InsecureSQLConnection.ql
@@ -14,9 +14,9 @@ import csharp
 import DataFlow::PathGraph
 
 /**
- * A `DataFlow::Configuration` for tracking `Strings passed to SqlConnectionStringBuilder` instances.
+ * A data flow configuration for tracking strings passed to `SqlConnection[StringBuilder]` instances.
  */
-class TaintTrackingConfiguration extends TaintTracking::Configuration {
+class TaintTrackingConfiguration extends DataFlow::Configuration {
   TaintTrackingConfiguration() { this = "TaintTrackingConfiguration" }
 
   override predicate isSource(DataFlow::Node source) {
diff --git a/csharp/ql/test/query-tests/Security Features/CWE-327/InsecureSQLConnection/InsecureSQLConnection.cs b/csharp/ql/test/query-tests/Security Features/CWE-327/InsecureSQLConnection/InsecureSQLConnection.cs
@@ -34,15 +34,15 @@ public void StringInBuilderProperty()
 
         public void TriggerThis()
         {
-            // BAD, Encrypt not specified [NOT DETECTED]
+            // BAD, Encrypt not specified
             SqlConnection conn = new SqlConnection("Server=myServerName\\myInstanceName;Database=myDataBase;User Id=myUsername;");
         }
 
         void Test4()
         {
             string connectString =
                 "Server=1.2.3.4;Database=Anything;UID=ab;Pwd=cd";
-            // BAD, Encrypt not specified [NOT DETECTED]
+            // BAD, Encrypt not specified
             SqlConnectionStringBuilder builder = new SqlConnectionStringBuilder(connectString);
             var conn = new SqlConnection(builder.ConnectionString);
         }
@@ -51,7 +51,7 @@ void Test5()
         {
             string connectString =
                 "Server=1.2.3.4;Database=Anything;UID=ab;Pwd=cd;Encrypt=false";
-            // BAD, Encrypt set to false [NOT DETECTED]
+            // BAD, Encrypt set to false
             SqlConnectionStringBuilder builder = new SqlConnectionStringBuilder(connectString);
             var conn = new SqlConnection(builder.ConnectionString);
         }
diff --git a/csharp/ql/test/query-tests/Security Features/CWE-327/InsecureSQLConnection/InsecureSQLConnection.expected b/csharp/ql/test/query-tests/Security Features/CWE-327/InsecureSQLConnection/InsecureSQLConnection.expected
@@ -0,0 +1,13 @@
+edges
+| InsecureSQLConnection.cs:44:17:44:64 | "Server=1.2.3.4;Database=Anything;UID=ab;Pwd=cd" : String | InsecureSQLConnection.cs:46:81:46:93 | access to local variable connectString |
+| InsecureSQLConnection.cs:53:17:53:78 | "Server=1.2.3.4;Database=Anything;UID=ab;Pwd=cd;Encrypt=false" : String | InsecureSQLConnection.cs:55:81:55:93 | access to local variable connectString |
+nodes
+| InsecureSQLConnection.cs:38:52:38:128 | "Server=myServerName\\myInstanceName;Database=myDataBase;User Id=myUsername;" | semmle.label | "Server=myServerName\\myInstanceName;Database=myDataBase;User Id=myUsername;" |
+| InsecureSQLConnection.cs:44:17:44:64 | "Server=1.2.3.4;Database=Anything;UID=ab;Pwd=cd" : String | semmle.label | "Server=1.2.3.4;Database=Anything;UID=ab;Pwd=cd" : String |
+| InsecureSQLConnection.cs:46:81:46:93 | access to local variable connectString | semmle.label | access to local variable connectString |
+| InsecureSQLConnection.cs:53:17:53:78 | "Server=1.2.3.4;Database=Anything;UID=ab;Pwd=cd;Encrypt=false" : String | semmle.label | "Server=1.2.3.4;Database=Anything;UID=ab;Pwd=cd;Encrypt=false" : String |
+| InsecureSQLConnection.cs:55:81:55:93 | access to local variable connectString | semmle.label | access to local variable connectString |
+#select
+| InsecureSQLConnection.cs:38:52:38:128 | "Server=myServerName\\myInstanceName;Database=myDataBase;User Id=myUsername;" | InsecureSQLConnection.cs:38:52:38:128 | "Server=myServerName\\myInstanceName;Database=myDataBase;User Id=myUsername;" | InsecureSQLConnection.cs:38:52:38:128 | "Server=myServerName\\myInstanceName;Database=myDataBase;User Id=myUsername;" | $@ flows to here and does not specify `Encrypt=True`. | InsecureSQLConnection.cs:38:52:38:128 | "Server=myServerName\\myInstanceName;Database=myDataBase;User Id=myUsername;" | Connection string |
+| InsecureSQLConnection.cs:46:81:46:93 | access to local variable connectString | InsecureSQLConnection.cs:44:17:44:64 | "Server=1.2.3.4;Database=Anything;UID=ab;Pwd=cd" : String | InsecureSQLConnection.cs:46:81:46:93 | access to local variable connectString | $@ flows to here and does not specify `Encrypt=True`. | InsecureSQLConnection.cs:44:17:44:64 | "Server=1.2.3.4;Database=Anything;UID=ab;Pwd=cd" | Connection string |
+| InsecureSQLConnection.cs:55:81:55:93 | access to local variable connectString | InsecureSQLConnection.cs:53:17:53:78 | "Server=1.2.3.4;Database=Anything;UID=ab;Pwd=cd;Encrypt=false" : String | InsecureSQLConnection.cs:55:81:55:93 | access to local variable connectString | $@ flows to here and does not specify `Encrypt=True`. | InsecureSQLConnection.cs:53:17:53:78 | "Server=1.2.3.4;Database=Anything;UID=ab;Pwd=cd;Encrypt=false" | Connection string |
diff --git a/csharp/ql/test/query-tests/Security Features/CWE-327/InsecureSQLConnection/InsecureSQLConnection.qlref b/csharp/ql/test/query-tests/Security Features/CWE-327/InsecureSQLConnection/InsecureSQLConnection.qlref
@@ -1 +1 @@
-Security Features/CWE-091/XMLInjection.ql
+Security Features/CWE-327/InsecureSQLConnection.ql

Original file line number	Diff line number	Diff line change
`@@ -34,15 +34,15 @@ public void StringInBuilderProperty()`
`34`	`34`
`35`	`35`	`public void TriggerThis()`
`36`	`36`	`{`
`37`		`- // BAD, Encrypt not specified [NOT DETECTED]`
	`37`	`+ // BAD, Encrypt not specified`
`38`	`38`	`SqlConnection conn = new SqlConnection("Server=myServerName\\myInstanceName;Database=myDataBase;User Id=myUsername;");`
`39`	`39`	`}`
`40`	`40`
`41`	`41`	`void Test4()`
`42`	`42`	`{`
`43`	`43`	`string connectString =`
`44`	`44`	`"Server=1.2.3.4;Database=Anything;UID=ab;Pwd=cd";`
`45`		`- // BAD, Encrypt not specified [NOT DETECTED]`
	`45`	`+ // BAD, Encrypt not specified`
`46`	`46`	`SqlConnectionStringBuilder builder = new SqlConnectionStringBuilder(connectString);`
`47`	`47`	`var conn = new SqlConnection(builder.ConnectionString);`
`48`	`48`	`}`
`@@ -51,7 +51,7 @@ void Test5()`
`51`	`51`	`{`
`52`	`52`	`string connectString =`
`53`	`53`	`"Server=1.2.3.4;Database=Anything;UID=ab;Pwd=cd;Encrypt=false";`
`54`		`- // BAD, Encrypt set to false [NOT DETECTED]`
	`54`	`+ // BAD, Encrypt set to false`
`55`	`55`	`SqlConnectionStringBuilder builder = new SqlConnectionStringBuilder(connectString);`
`56`	`56`	`var conn = new SqlConnection(builder.ConnectionString);`
`57`	`57`	`}`
Original file line number	Diff line number	Diff line change
`@@ -1 +1 @@`
`1`		`-Security Features/CWE-091/XMLInjection.ql`
	`1`	`+Security Features/CWE-327/InsecureSQLConnection.ql`