SecondaryCommandInjection to RemoteCommandExecution, change RemoteCommandExecution to module like SystemCommandExecution module

Author: am0o0

python/ql/src/experimental/semmle/python/Concepts.qll

@@ -15,12 +15,23 @@ private import semmle.python.dataflow.new.TaintTracking
 private import experimental.semmle.python.Frameworks
 private import semmle.python.Concepts
 
-/**
- * A data-flow node that responsible for a command that can be executed on a secondary remote system,
- *
- * Extend this class to model new APIs.
- */
-abstract class SecondaryCommandInjection extends DataFlow::Node { }
+/** Provides classes for modeling remote server command execution related APIs. */
+module RemoteCommandExecution {
+  /**
+   * A data-flow node that executes an operating system command,
+   * on a remote server likely by SSH connections.
+   *
+   * Extend this class to model new APIs. If you want to refine existing API models,
+   * extend `SystemCommandExecution` instead.
+   */
+  abstract class Range extends DataFlow::Node {
+    /** Gets the argument that specifies the command to be executed. */
+    abstract DataFlow::Node getCommand();
+
+    /** Holds if a shell interprets `arg`. */
+    predicate isShellInterpreted(DataFlow::Node arg) { none() }
+  }
+}
 
 /** Provides classes for modeling copying file related APIs. */
 module CopyFile {

python/ql/src/experimental/semmle/python/frameworks/AsyncSsh.qll

@@ -22,16 +22,9 @@ private module Asyncssh {
   /**
    * A `run` method responsible for executing commands on remote secondary servers.
    */
-  class AsyncsshRun extends SecondaryCommandInjection {
-    AsyncsshRun() {
-      this =
-        asyncssh()
-            .getMember("connect")
-            .getReturn()
-            .getMember("run")
-            .getACall()
-            .getParameter(0, "command")
-            .asSink()
-    }
+  class AsyncsshRun extends RemoteCommandExecution::Range, API::CallNode {
+    AsyncsshRun() { this = asyncssh().getMember("connect").getReturn().getMember("run").getACall() }
+
+    override DataFlow::Node getCommand() { result = this.getParameter(0, "command").asSink() }
   }
 }

python/ql/src/experimental/semmle/python/frameworks/Netmiko.qll

@@ -29,21 +29,27 @@ private module Netmiko {
   /**
    * The `send_*` methods responsible for executing commands on remote secondary servers.
    */
-  class NetmikoSendCommand extends SecondaryCommandInjection {
+  class NetmikoSendCommand extends RemoteCommandExecution::Range, API::CallNode {
+    boolean isMultiline;
+
     NetmikoSendCommand() {
       this =
         netmikoConnectHandler()
             .getMember(["send_command", "send_command_expect", "send_command_timing"])
-            .getACall()
-            .getParameter(0, "command_string")
-            .asSink()
+            .getACall() and
+      isMultiline = false
       or
       this =
-        netmikoConnectHandler()
-            .getMember(["send_multiline", "send_multiline_timing"])
-            .getACall()
-            .getParameter(0, "commands")
-            .asSink()
+        netmikoConnectHandler().getMember(["send_multiline", "send_multiline_timing"]).getACall() and
+      isMultiline = true
+    }
+
+    override DataFlow::Node getCommand() {
+      result = this.getParameter(0, "command_string").asSink() and
+      isMultiline = false
+      or
+      result = this.getParameter(0, "commands").asSink() and
+      isMultiline = true
     }
   }
 }

python/ql/src/experimental/semmle/python/frameworks/Paramiko.qll

@@ -27,10 +27,9 @@ private module Paramiko {
   /**
    * The `exec_command` of `paramiko.SSHClient` class execute command on ssh target server
    */
-  class ParamikoExecCommand extends SecondaryCommandInjection {
-    ParamikoExecCommand() {
-      this =
-        paramikoClient().getMember("exec_command").getACall().getParameter(0, "command").asSink()
-    }
+  class ParamikoExecCommand extends RemoteCommandExecution::Range, API::CallNode {
+    ParamikoExecCommand() { this = paramikoClient().getMember("exec_command").getACall() }
+
+    override DataFlow::Node getCommand() { result = this.getParameter(0, "command").asSink() }
   }
 }

python/ql/src/experimental/semmle/python/frameworks/Pexpect.qll

@@ -17,7 +17,7 @@ private module Pexpect {
    * The calls to `pexpect.pxssh.pxssh` functions that execute commands
    * See https://pexpect.readthedocs.io/en/stable/api/pxssh.html
    */
-  class PexpectCommandExec extends SecondaryCommandInjection {
+  class PexpectCommandExec extends RemoteCommandExecution::Range, API::CallNode {
     PexpectCommandExec() {
       this =
         API::moduleImport("pexpect")
@@ -26,8 +26,8 @@ private module Pexpect {
             .getReturn()
             .getMember(["send", "sendline"])
             .getACall()
-            .getParameter(0, "s")
-            .asSink()
     }
+
+    override DataFlow::Node getCommand() { result = this.getParameter(0, "s").asSink() }
   }
 }

python/ql/src/experimental/semmle/python/frameworks/Scrapli.qll

@@ -32,7 +32,7 @@ private module Scrapli {
   /**
    * A `send_command` method responsible for executing commands on remote secondary servers.
    */
-  class ScrapliSendCommand extends SecondaryCommandInjection {
+  class ScrapliSendCommand extends RemoteCommandExecution::Range, API::CallNode {
     ScrapliSendCommand() {
       this =
         scrapliCore()
@@ -44,26 +44,13 @@ private module Scrapli {
             .getReturn()
             .getMember("send_command")
             .getACall()
-            .getParameter(0, "command")
-            .asSink()
       or
-      this =
-        scrapli()
-            .getMember("Scrapli")
-            .getReturn()
-            .getMember("send_command")
-            .getACall()
-            .getParameter(0, "command")
-            .asSink()
+      this = scrapli().getMember("Scrapli").getReturn().getMember("send_command").getACall()
       or
       this =
-        scrapliDriver()
-            .getMember("GenericDriver")
-            .getReturn()
-            .getMember("send_command")
-            .getACall()
-            .getParameter(0, "command")
-            .asSink()
+        scrapliDriver().getMember("GenericDriver").getReturn().getMember("send_command").getACall()
     }
+
+    override DataFlow::Node getCommand() { result = this.getParameter(0, "command").asSink() }
   }
 }

python/ql/src/experimental/semmle/python/frameworks/Ssh2.qll

@@ -29,16 +29,11 @@ private module Ssh2 {
   /**
    * An `execute` method responsible for executing commands on remote secondary servers.
    */
-  class Ssh2Execute extends SecondaryCommandInjection {
+  class Ssh2Execute extends RemoteCommandExecution::Range, API::CallNode {
     Ssh2Execute() {
-      this =
-        ssh2Session()
-            .getMember("open_session")
-            .getReturn()
-            .getMember("execute")
-            .getACall()
-            .getParameter(0, "command")
-            .asSink()
+      this = ssh2Session().getMember("open_session").getReturn().getMember("execute").getACall()
     }
+
+    override DataFlow::Node getCommand() { result = this.getParameter(0, "command").asSink() }
   }
 }

python/ql/src/experimental/semmle/python/frameworks/Twisted.qll

@@ -19,7 +19,7 @@ private module Twisted {
   /**
    * The `newConnection` and `existingConnection` functions of `twisted.conch.endpoints.SSHCommandClientEndpoint` class execute command on ssh target server
    */
-  class ParamikoExecCommand extends SecondaryCommandInjection {
+  class ParamikoExecCommand extends RemoteCommandExecution::Range, API::CallNode {
     ParamikoExecCommand() {
       this =
         API::moduleImport("twisted")
@@ -28,8 +28,8 @@ private module Twisted {
             .getMember("SSHCommandClientEndpoint")
             .getMember(["newConnection", "existingConnection"])
             .getACall()
-            .getParameter(1, "command")
-            .asSink()
     }
+
+    override DataFlow::Node getCommand() { result = this.getParameter(1, "command").asSink() }
   }
 }