Merge pull request github#16523 from microsoft/jb1/chanely-insecure-sql-connection

hvitved · web-flow · commit 331f67683274 · 2024-06-06T12:58:14.000+02:00
Adding case to InsecureSQLConnection.ql when Encrypt set in initializer
diff --git a/csharp/ql/src/Security Features/CWE-327/InsecureSQLConnection.ql b/csharp/ql/src/Security Features/CWE-327/InsecureSQLConnection.ql
@@ -32,6 +32,11 @@ module InsecureSqlConnectionConfig implements DataFlow::ConfigSig {
         oc.getType().getName() = "SqlConnectionStringBuilder"
         or
         oc.getType().getName() = "SqlConnection"
+      ) and
+      not exists(MemberInitializer mi |
+        mi = oc.getInitializer().(ObjectInitializer).getAMemberInitializer() and
+        mi.getLValue().(PropertyAccess).getTarget().getName() = "Encrypt" and
+        mi.getRValue().(BoolLiteral).getValue() = "true"
       )
     )
   }
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
@@ -29,8 +29,14 @@ public void StringInBuilderProperty()
             conBuilder.Encrypt = true;
             SqlConnection conn = new SqlConnection();
             conn.ConnectionString = conBuilder.ToString();
+        }
 
+        public void StringInInitializer()
+        {
+            string connectString = "Server=1.2.3.4;Database=Anything;UID=ab;Pwd=cd;Encrypt=false";
+            SqlConnectionStringBuilder conBuilder = new SqlConnectionStringBuilder(connectString) { Encrypt = true};
         }
+        
 
         public void TriggerThis()
         {
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
@@ -1,18 +1,18 @@
 edges
-| InsecureSQLConnection.cs:43:20:43:32 | access to local variable connectString : String | InsecureSQLConnection.cs:46:81:46:93 | access to local variable connectString | provenance |  |
-| InsecureSQLConnection.cs:44:17:44:64 | "Server=1.2.3.4;Database=Anything;UID=ab;Pwd=cd" : String | InsecureSQLConnection.cs:43:20:43:32 | access to local variable connectString : String | provenance |  |
-| InsecureSQLConnection.cs:52:20:52:32 | access to local variable connectString : String | InsecureSQLConnection.cs:55:81:55:93 | access to local variable connectString | provenance |  |
-| InsecureSQLConnection.cs:53:17:53:78 | "Server=1.2.3.4;Database=Anything;UID=ab;Pwd=cd;Encrypt=false" : String | InsecureSQLConnection.cs:52:20:52:32 | access to local variable connectString : String | provenance |  |
+| InsecureSQLConnection.cs:49:20:49:32 | access to local variable connectString : String | InsecureSQLConnection.cs:52:81:52:93 | access to local variable connectString | provenance |  |
+| InsecureSQLConnection.cs:50:17:50:64 | "Server=1.2.3.4;Database=Anything;UID=ab;Pwd=cd" : String | InsecureSQLConnection.cs:49:20:49:32 | access to local variable connectString : String | provenance |  |
+| InsecureSQLConnection.cs:58:20:58:32 | access to local variable connectString : String | InsecureSQLConnection.cs:61:81:61:93 | access to local variable connectString | provenance |  |
+| InsecureSQLConnection.cs:59:17:59:78 | "Server=1.2.3.4;Database=Anything;UID=ab;Pwd=cd;Encrypt=false" : String | InsecureSQLConnection.cs:58:20:58:32 | access to local variable connectString : String | provenance |  |
 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:43:20:43:32 | access to local variable connectString : String | semmle.label | access to local variable connectString : String |
-| 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:52:20:52:32 | access to local variable connectString : String | semmle.label | access to local variable connectString : String |
-| 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 |
+| InsecureSQLConnection.cs:44:52:44:128 | "Server=myServerName\\myInstanceName;Database=myDataBase;User Id=myUsername;" | semmle.label | "Server=myServerName\\myInstanceName;Database=myDataBase;User Id=myUsername;" |
+| InsecureSQLConnection.cs:49:20:49:32 | access to local variable connectString : String | semmle.label | access to local variable connectString : String |
+| InsecureSQLConnection.cs:50:17:50: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:52:81:52:93 | access to local variable connectString | semmle.label | access to local variable connectString |
+| InsecureSQLConnection.cs:58:20:58:32 | access to local variable connectString : String | semmle.label | access to local variable connectString : String |
+| InsecureSQLConnection.cs:59:17:59: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:61:81:61:93 | access to local variable connectString | semmle.label | access to local variable connectString |
 subpaths
 #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 this SQL connection 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 this SQL connection 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 this SQL connection 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 |
+| InsecureSQLConnection.cs:44:52:44:128 | "Server=myServerName\\myInstanceName;Database=myDataBase;User Id=myUsername;" | InsecureSQLConnection.cs:44:52:44:128 | "Server=myServerName\\myInstanceName;Database=myDataBase;User Id=myUsername;" | InsecureSQLConnection.cs:44:52:44:128 | "Server=myServerName\\myInstanceName;Database=myDataBase;User Id=myUsername;" | $@ flows to this SQL connection and does not specify `Encrypt=True`. | InsecureSQLConnection.cs:44:52:44:128 | "Server=myServerName\\myInstanceName;Database=myDataBase;User Id=myUsername;" | Connection string |
+| InsecureSQLConnection.cs:52:81:52:93 | access to local variable connectString | InsecureSQLConnection.cs:50:17:50:64 | "Server=1.2.3.4;Database=Anything;UID=ab;Pwd=cd" : String | InsecureSQLConnection.cs:52:81:52:93 | access to local variable connectString | $@ flows to this SQL connection and does not specify `Encrypt=True`. | InsecureSQLConnection.cs:50:17:50:64 | "Server=1.2.3.4;Database=Anything;UID=ab;Pwd=cd" | Connection string |
+| InsecureSQLConnection.cs:61:81:61:93 | access to local variable connectString | InsecureSQLConnection.cs:59:17:59:78 | "Server=1.2.3.4;Database=Anything;UID=ab;Pwd=cd;Encrypt=false" : String | InsecureSQLConnection.cs:61:81:61:93 | access to local variable connectString | $@ flows to this SQL connection and does not specify `Encrypt=True`. | InsecureSQLConnection.cs:59:17:59:78 | "Server=1.2.3.4;Database=Anything;UID=ab;Pwd=cd;Encrypt=false" | Connection string |

Original file line number	Diff line number	Diff line change
`@@ -32,6 +32,11 @@ module InsecureSqlConnectionConfig implements DataFlow::ConfigSig {`
`32`	`32`	`oc.getType().getName() = "SqlConnectionStringBuilder"`
`33`	`33`	`or`
`34`	`34`	`oc.getType().getName() = "SqlConnection"`
	`35`	`+ ) and`
	`36`	`+ not exists(MemberInitializer mi \|`
	`37`	`+ mi = oc.getInitializer().(ObjectInitializer).getAMemberInitializer() and`
	`38`	`+ mi.getLValue().(PropertyAccess).getTarget().getName() = "Encrypt" and`
	`39`	`+ mi.getRValue().(BoolLiteral).getValue() = "true"`
`35`	`40`	`)`
`36`	`41`	`)`
`37`	`42`	`}`
Original file line number	Diff line number	Diff line change
`@@ -29,8 +29,14 @@ public void StringInBuilderProperty()`
`29`	`29`	`conBuilder.Encrypt = true;`
`30`	`30`	`SqlConnection conn = new SqlConnection();`
`31`	`31`	`conn.ConnectionString = conBuilder.ToString();`
	`32`	`+ }`
`32`	`33`
	`34`	`+ public void StringInInitializer()`
	`35`	`+ {`
	`36`	`+ string connectString = "Server=1.2.3.4;Database=Anything;UID=ab;Pwd=cd;Encrypt=false";`
	`37`	`+ SqlConnectionStringBuilder conBuilder = new SqlConnectionStringBuilder(connectString) { Encrypt = true};`
`33`	`38`	`}`
	`39`	`+`
`34`	`40`
`35`	`41`	`public void TriggerThis()`
`36`	`42`	`{`