feat: add codeql exec wrapper sinks (#3037)

* add codeql exec wrapper sinks

* test codeql detection

* Revert "test codeql detection"

This reverts commit 539c9051f2.

codeql/acn-addipamconfig.ql

@@ -11,18 +11,7 @@
 
 // Detect inputs from CNS add ipam result / CNS multitenancy ipam add result to command injection
 import go
-
+import lib.ACN
-private class Sink extends DataFlow2::Node {
-  Sink() {
-    exists(DataFlow::CallNode c |
-      c.getTarget().hasQualifiedName("os/exec", "CommandContext") and
-      (c.getArgument(2) = this or c.getArgument(1) = this)
-      or
-      c.getTarget().hasQualifiedName("os/exec", "Command") and
-      (c.getArgument(0) = this or c.getArgument(1) = this)
-    )
-  }
-}
 
 private class Source extends DataFlow2::Node {
   Source() {
@@ -38,7 +27,7 @@ private class Source extends DataFlow2::Node {
 }
 
 module MyConfiguration implements DataFlow::ConfigSig {
-  predicate isSink(DataFlow::Node sink) { sink instanceof Sink }
+  predicate isSink(DataFlow::Node sink) { sink instanceof ACN::CommandSink }
 
   predicate isSource(DataFlow::Node source) { source instanceof Source }
 }

codeql/acn-cni-args.ql

@@ -11,18 +11,7 @@
 
 // Detect inputs from CNI ARGS to command injection
 import go
-
+import lib.ACN
-private class Sink extends DataFlow2::Node {
-  Sink() {
-    exists(DataFlow::CallNode c |
-      c.getTarget().hasQualifiedName("os/exec", "CommandContext") and
-      (c.getArgument(2) = this or c.getArgument(1) = this)
-      or
-      c.getTarget().hasQualifiedName("os/exec", "Command") and
-      (c.getArgument(0) = this or c.getArgument(1) = this)
-    )
-  }
-}
 
 private class Source extends DataFlow2::Node {
   Source() {
@@ -44,7 +33,7 @@ private class Source extends DataFlow2::Node {
 }
 
 module MyConfiguration implements DataFlow::ConfigSig {
-  predicate isSink(DataFlow::Node sink) { sink instanceof Sink }
+  predicate isSink(DataFlow::Node sink) { sink instanceof ACN::CommandSink }
 
   predicate isSource(DataFlow::Node source) { source instanceof Source }
 }

codeql/acn-cns-invoker.ql

@@ -12,18 +12,7 @@
 // Detect inputs from CNS Invoker to command injection
 // Does not detect flow to outside the enclosed method (which is why we analyze addIpamInvoker's results too)
 import go
-
+import lib.ACN
-private class Sink extends DataFlow2::Node {
-  Sink() {
-    exists(DataFlow::CallNode c |
-      c.getTarget().hasQualifiedName("os/exec", "CommandContext") and
-      (c.getArgument(2) = this or c.getArgument(1) = this)
-      or
-      c.getTarget().hasQualifiedName("os/exec", "Command") and
-      (c.getArgument(0) = this or c.getArgument(1) = this)
-    )
-  }
-}
 
 private class Source extends DataFlow2::Node {
   Source() {
@@ -45,7 +34,7 @@ private class Source extends DataFlow2::Node {
 }
 
 module MyConfiguration implements DataFlow::ConfigSig {
-  predicate isSink(DataFlow::Node sink) { sink instanceof Sink }
+  predicate isSink(DataFlow::Node sink) { sink instanceof ACN::CommandSink }
 
   predicate isSource(DataFlow::Node source) { source instanceof Source }
 }

codeql/acn-decode.ql

@@ -11,18 +11,7 @@
 
 // Detect flow from the DECODE method (which decodes http requests) to a command execution
 import go
-
+import lib.ACN
-private class Sink extends DataFlow2::Node {
-  Sink() {
-    exists(DataFlow::CallNode c |
-      c.getTarget().hasQualifiedName("os/exec", "CommandContext") and
-      (c.getArgument(2) = this or c.getArgument(1) = this)
-      or
-      c.getTarget().hasQualifiedName("os/exec", "Command") and
-      (c.getArgument(0) = this or c.getArgument(1) = this)
-    )
-  }
-}
 
 private class Source extends DataFlow2::Node {
   Source() {
@@ -34,7 +23,7 @@ private class Source extends DataFlow2::Node {
 }
 
 module MyConfiguration implements DataFlow::ConfigSig {
-  predicate isSink(DataFlow::Node sink) { sink instanceof Sink }
+  predicate isSink(DataFlow::Node sink) { sink instanceof ACN::CommandSink }
 
   predicate isSource(DataFlow::Node source) { source instanceof Source }
 }

codeql/lib/ACN.qll

@@ -0,0 +1,35 @@
+import go
+
+module ACN {
+  class CommandSink extends DataFlow2::Node {
+    CommandSink() {
+      exists(DataFlow::CallNode c, Method m |
+        (
+          // Detect dangerous usage of command wrappers with the command in the 0th arg position
+          (
+            m.hasQualifiedName("github.com/Azure/azure-container-networking/platform", "execClient",
+              "ExecuteRawCommand") or
+            m.hasQualifiedName("github.com/Azure/azure-container-networking/platform", "execClient",
+              "ExecutePowershellCommand")
+          ) and
+          c.getArgument(0) = this
+          or
+          // Detect dangerous usage of command wrappers with the command in the 1st arg position
+          m.hasQualifiedName("github.com/Azure/azure-container-networking/platform", "execClient",
+            "ExecutePowershellCommandWithContext") and
+          c.getArgument(1) = this
+        ) and
+        c = m.getACall()
+        or
+        // Detect dangerous calls directly to os exec
+        (
+          c.getTarget().hasQualifiedName("os/exec", "CommandContext") and
+          (c.getArgument(2) = this or c.getArgument(1) = this)
+          or
+          c.getTarget().hasQualifiedName("os/exec", "Command") and
+          (c.getArgument(0) = this or c.getArgument(1) = this)
+        )
+      )
+    }
+  }
+}