Merge pull request #74 from microsoft/users/chanely/insecure-sql-connection-versioncheck

Update to insecure sql connection to check for version

Author: chanel-y

csharp/ql/src/Security Features/CWE-327/InsecureSQLConnection.ql

@@ -13,33 +13,50 @@
 import csharp
 import InsecureSqlConnection::PathGraph
 
-/**
- * A data flow configuration for tracking strings passed to `SqlConnection[StringBuilder]` instances.
- */
-module InsecureSqlConnectionConfig implements DataFlow::ConfigSig {
-  predicate isSource(DataFlow::Node source) {
-    exists(string s | s = source.asExpr().(StringLiteral).getValue().toLowerCase() |
-      s.matches("%encrypt=false%")
-      or
-      not s.matches("%encrypt=%")
+class Source extends DataFlow::Node {
+  string sourcestring;
+
+  Source() {
+    sourcestring = this.asExpr().(StringLiteral).getValue().toLowerCase() and
+    (
+      not sourcestring.matches("%encrypt=%") or
+      sourcestring.matches("%encrypt=false%")
     )
   }
 
-  predicate isSink(DataFlow::Node sink) {
+  predicate setsEncryptFalse() { sourcestring.matches("%encrypt=false%") }
+}
+
+class Sink extends DataFlow::Node {
+  Version version;
+
+  Sink() {
     exists(ObjectCreation oc |
-      oc.getRuntimeArgument(0) = sink.asExpr() and
+      oc.getRuntimeArgument(0) = this.asExpr() and
       (
         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"
-      )
+      version = oc.getType().getALocation().(Assembly).getVersion()
     )
   }
+
+  predicate isEncryptedByDefault() { version.compareTo("4.0") >= 0 }
+}
+
+predicate isEncryptTrue(Source source, Sink sink) {
+  sink.isEncryptedByDefault() and
+  not source.setsEncryptFalse()
+}
+
+/**
+ * A data flow configuration for tracking strings passed to `SqlConnection[StringBuilder]` instances.
+ */
+module InsecureSqlConnectionConfig implements DataFlow::ConfigSig {
+  predicate isSource(DataFlow::Node source) { source instanceof Source }
+
+  predicate isSink(DataFlow::Node sink) { sink instanceof Sink }
 }
 
 /**
@@ -48,7 +65,9 @@ module InsecureSqlConnectionConfig implements DataFlow::ConfigSig {
 module InsecureSqlConnection = DataFlow::Global<InsecureSqlConnectionConfig>;
 
 from InsecureSqlConnection::PathNode source, InsecureSqlConnection::PathNode sink
-where InsecureSqlConnection::flowPath(source, sink)
+where
+  InsecureSqlConnection::flowPath(source, sink) and
+  not isEncryptTrue(source.getNode().(Source), sink.getNode().(Sink))
 select sink.getNode(), source, sink,
   "$@ flows to this SQL connection and does not specify `Encrypt=True`.", source.getNode(),
   "Connection string"