rename secondary to remote :), complete the previous commit changes

am0o0 · am0o0 · commit b9edcb794393 · 2024-05-29T16:47:37.000+02:00
diff --git a/python/ql/src/experimental/Security/CWE-074/remoteCommandExecution/RemoteCommandExecution.qhelp b/python/ql/src/experimental/Security/CWE-074/remoteCommandExecution/RemoteCommandExecution.qhelp
@@ -2,10 +2,10 @@
 <qhelp>
     <overview>
         <p>
-            Allowing users to execute arbitrary commands using an SSH connection on a secondary server can lead to security issues unless you implement proper authorization.
+            Allowing users to execute arbitrary commands using an SSH connection on a remote server can lead to security issues unless you implement proper authorization.
         </p>
         <p>
-            Assume that you connect to a secondary system via SSH connection from your main or local server that accepts user-controlled data and has interaction with users that you don't trust, passing these data to SSH API as a part of a command that will be executed on a secondary remote server can lead to security issues. You should consider proper authorization rules very carefully.
+            Assume that you connect to a remote system via SSH connection from your main or local server that accepts user-controlled data and has interaction with users that you don't trust, passing these data to SSH API as a part of a command that will be executed on a secondary remote server can lead to security issues. You should consider proper authorization rules very carefully.
         </p>
     </overview>
     <recommendation>
diff --git a/python/ql/src/experimental/Security/CWE-074/remoteCommandExecution/RemoteCommandExecution.ql b/python/ql/src/experimental/Security/CWE-074/remoteCommandExecution/RemoteCommandExecution.ql
@@ -12,10 +12,10 @@
  */
 
 import python
-import experimental.semmle.python.security.SecondaryServerCmdInjection
-import SecondaryCommandInjectionFlow::PathGraph
+import experimental.semmle.python.security.RemoteCommandExecution
+import RemoteCommandExecutionFlow::PathGraph
 
-from SecondaryCommandInjectionFlow::PathNode source, SecondaryCommandInjectionFlow::PathNode sink
-where SecondaryCommandInjectionFlow::flowPath(source, sink)
+from RemoteCommandExecutionFlow::PathNode source, RemoteCommandExecutionFlow::PathNode sink
+where RemoteCommandExecutionFlow::flowPath(source, sink)
 select sink.getNode(), source, sink, "This code execution depends on a $@.", source.getNode(),
   "a user-provided value"
diff --git a/python/ql/src/experimental/Security/CWE-074/remoteCommandExecution/paramikoBad.py b/python/ql/src/experimental/Security/CWE-074/remoteCommandExecution/paramikoBad.py
diff --git a/python/ql/src/experimental/Security/CWE-074/remoteCommandExecution/paramikoGood.py b/python/ql/src/experimental/Security/CWE-074/remoteCommandExecution/paramikoGood.py
diff --git a/python/ql/src/experimental/semmle/python/Concepts.qll b/python/ql/src/experimental/semmle/python/Concepts.qll
@@ -15,14 +15,29 @@ private import semmle.python.dataflow.new.TaintTracking
 private import experimental.semmle.python.Frameworks
 private import semmle.python.Concepts
 
-/** Provides classes for modeling remote server command execution related APIs. */
+/**
+ * A data-flow node that executes an operating system command,
+ * on a remote server likely by SSH connections.
+ *
+ * Extend this class to refine existing API models. If you want to model new APIs,
+ * extend `RemoteCommandExecution::Range` instead.
+ */
+class RemoteCommandExecution extends DataFlow::Node instanceof RemoteCommandExecution::Range {
+  /** Holds if a shell interprets `arg`. */
+  predicate isShellInterpreted(DataFlow::Node arg) { super.isShellInterpreted(arg) }
+
+  /** Gets the argument that specifies the command to be executed. */
+  DataFlow::Node getCommand() { result = super.getCommand() }
+}
+
+/** Provides classes for modeling new remote server command execution 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.
+   * extend `RemoteCommandExecution` instead.
    */
   abstract class Range extends DataFlow::Node {
     /** Gets the argument that specifies the command to be executed. */
diff --git a/python/ql/src/experimental/semmle/python/security/RemoteCommandExecution.qll b/python/ql/src/experimental/semmle/python/security/RemoteCommandExecution.qll
@@ -6,11 +6,11 @@ import semmle.python.dataflow.new.internal.DataFlowPublic
 import codeql.util.Unit
 import experimental.semmle.python.Concepts
 
-module SecondaryCommandInjectionConfig implements DataFlow::ConfigSig {
+module RemoteCommandExecutionConfig implements DataFlow::ConfigSig {
   predicate isSource(DataFlow::Node source) { source instanceof RemoteFlowSource }
 
-  predicate isSink(DataFlow::Node sink) { sink instanceof SecondaryCommandInjection }
+  predicate isSink(DataFlow::Node sink) { sink = any(RemoteCommandExecution rce).getCommand() }
 }
 
 /** Global taint-tracking for detecting "secondary server command injection" vulnerabilities. */
-module SecondaryCommandInjectionFlow = TaintTracking::Global<SecondaryCommandInjectionConfig>;
+module RemoteCommandExecutionFlow = TaintTracking::Global<RemoteCommandExecutionConfig>;
diff --git a/python/ql/test/experimental/query-tests/Security/CWE-074-RemoteCommandExecution/AsyncSsh.py b/python/ql/test/experimental/query-tests/Security/CWE-074-RemoteCommandExecution/AsyncSsh.py
@@ -14,6 +14,6 @@
 @app.get("/bad1")
 async def bad1(cmd: str):
     async with asyncssh.connect('localhost') as conn:
-        result = await conn.run(cmd, check=True) # $ result=BAD getSecondaryCommand=cmd
+        result = await conn.run(cmd, check=True) # $ result=BAD getRemoteCommand=cmd
         print(result.stdout, end='')
     return {"success": "Dangerous"}
diff --git a/python/ql/test/experimental/query-tests/Security/CWE-074-RemoteCommandExecution/ConceptsTest.expected b/python/ql/test/experimental/query-tests/Security/CWE-074-RemoteCommandExecution/ConceptsTest.expected
diff --git a/python/ql/test/experimental/query-tests/Security/CWE-074-RemoteCommandExecution/ConceptsTest.ql b/python/ql/test/experimental/query-tests/Security/CWE-074-RemoteCommandExecution/ConceptsTest.ql
@@ -6,16 +6,16 @@ private import semmle.python.dataflow.new.internal.PrintNode
 import experimental.semmle.python.Concepts
 
 module SystemCommandExecutionTest implements TestSig {
-  string getARelevantTag() { result = "getSecondaryCommand" }
+  string getARelevantTag() { result = "getRemoteCommand" }
 
   predicate hasActualResult(Location location, string element, string tag, string value) {
     exists(location.getFile().getRelativePath()) and
-    exists(SecondaryCommandInjection sci, DataFlow::Node command |
-      command = sci and
+    exists(RemoteCommandExecution sci, DataFlow::Node command |
+      command = sci.getCommand() and
       location = command.getLocation() and
       element = command.toString() and
       value = prettyNodeForInlineTest(command) and
-      tag = "getSecondaryCommand"
+      tag = "getRemoteCommand"
     )
   }
 }
diff --git a/python/ql/test/experimental/query-tests/Security/CWE-074-RemoteCommandExecution/DataflowQueryTest.expected b/python/ql/test/experimental/query-tests/Security/CWE-074-RemoteCommandExecution/DataflowQueryTest.expected
diff --git a/python/ql/test/experimental/query-tests/Security/CWE-074-RemoteCommandExecution/DataflowQueryTest.ql b/python/ql/test/experimental/query-tests/Security/CWE-074-RemoteCommandExecution/DataflowQueryTest.ql
@@ -0,0 +1,4 @@
+import python
+import TestUtilities.dataflow.DataflowQueryTest
+import experimental.semmle.python.security.RemoteCommandExecution
+import FromTaintTrackingConfig<RemoteCommandExecutionConfig>
diff --git a/python/ql/test/experimental/query-tests/Security/CWE-074-RemoteCommandExecution/Netmiko.py b/python/ql/test/experimental/query-tests/Security/CWE-074-RemoteCommandExecution/Netmiko.py
@@ -17,9 +17,9 @@
 @app.get("/bad1")
 async def bad1(cmd: str):
     net_connect = ConnectHandler(**cisco_881)
-    net_connect.send_command(command_string=cmd) # $ result=BAD getSecondaryCommand=cmd
-    net_connect.send_command_expect(command_string=cmd) # $ result=BAD getSecondaryCommand=cmd
-    net_connect.send_command_timing(command_string=cmd) # $ result=BAD getSecondaryCommand=cmd
-    net_connect.send_multiline(commands=[[cmd, "expect"]]) # $ result=BAD getSecondaryCommand=List
-    net_connect.send_multiline_timing(commands=cmd) # $ result=BAD getSecondaryCommand=cmd
+    net_connect.send_command(command_string=cmd) # $ result=BAD getRemoteCommand=cmd
+    net_connect.send_command_expect(command_string=cmd) # $ result=BAD getRemoteCommand=cmd
+    net_connect.send_command_timing(command_string=cmd) # $ result=BAD getRemoteCommand=cmd
+    net_connect.send_multiline(commands=[[cmd, "expect"]]) # $ result=BAD getRemoteCommand=List
+    net_connect.send_multiline_timing(commands=cmd) # $ result=BAD getRemoteCommand=cmd
     return {"success": "Dangerous"}
diff --git a/python/ql/test/experimental/query-tests/Security/CWE-074-RemoteCommandExecution/Pexpect.py b/python/ql/test/experimental/query-tests/Security/CWE-074-RemoteCommandExecution/Pexpect.py
@@ -13,9 +13,9 @@
 
 @app.get("/bad1")
 async def bad1(cmd: str):
-    ssh.send(cmd)  # $ result=BAD getSecondaryCommand=cmd
+    ssh.send(cmd)  # $ result=BAD getRemoteCommand=cmd
     ssh.prompt()
-    ssh.sendline(cmd)  # $ result=BAD getSecondaryCommand=cmd
+    ssh.sendline(cmd)  # $ result=BAD getRemoteCommand=cmd
     ssh.prompt()
     ssh.logout()
     return {"success": stdout}
diff --git a/python/ql/test/experimental/query-tests/Security/CWE-074-RemoteCommandExecution/RemoteCommandExecution.expected b/python/ql/test/experimental/query-tests/Security/CWE-074-RemoteCommandExecution/RemoteCommandExecution.expected
diff --git a/python/ql/test/experimental/query-tests/Security/CWE-074-RemoteCommandExecution/RemoteCommandExecution.qlref b/python/ql/test/experimental/query-tests/Security/CWE-074-RemoteCommandExecution/RemoteCommandExecution.qlref
@@ -0,0 +1 @@
+experimental/Security/CWE-074/remoteCommandExecution/RemoteCommandExecution.ql
diff --git a/python/ql/test/experimental/query-tests/Security/CWE-074-RemoteCommandExecution/Scrapli.py b/python/ql/test/experimental/query-tests/Security/CWE-074-RemoteCommandExecution/Scrapli.py
@@ -21,19 +21,19 @@ async def bad1(cmd: str):
     }
     driver = AsyncIOSXEDriver
     async with driver(**dev_connect) as conn:
-        output = await conn.send_command(cmd) # $ result=BAD getSecondaryCommand=cmd
+        output = await conn.send_command(cmd) # $ result=BAD getRemoteCommand=cmd
     driver = AsyncIOSXRDriver
     async with driver(**dev_connect) as conn:
-        output = await conn.send_command(cmd) # $ result=BAD getSecondaryCommand=cmd
+        output = await conn.send_command(cmd) # $ result=BAD getRemoteCommand=cmd
     driver = AsyncNXOSDriver
     async with driver(**dev_connect) as conn:
-        output = await conn.send_command(cmd) # $ result=BAD getSecondaryCommand=cmd
+        output = await conn.send_command(cmd) # $ result=BAD getRemoteCommand=cmd
     driver = AsyncEOSDriver
     async with driver(**dev_connect) as conn:
-        output = await conn.send_command(cmd) # $ result=BAD getSecondaryCommand=cmd
+        output = await conn.send_command(cmd) # $ result=BAD getRemoteCommand=cmd
     driver = AsyncJunosDriver
     async with driver(**dev_connect) as conn:
-        output = await conn.send_command(cmd) # $ result=BAD getSecondaryCommand=cmd
+        output = await conn.send_command(cmd) # $ result=BAD getRemoteCommand=cmd
     return {"success": "Dangerous"}
 
 @app.get("/bad1")
@@ -48,19 +48,19 @@ def bad2(cmd: str):
     }
     driver = NXOSDriver
     with driver(**dev_connect) as conn:
-        output = conn.send_command(cmd) # $ result=BAD getSecondaryCommand=cmd
+        output = conn.send_command(cmd) # $ result=BAD getRemoteCommand=cmd
     driver = IOSXRDriver
     with driver(**dev_connect) as conn:
-        output = conn.send_command(cmd) # $ result=BAD getSecondaryCommand=cmd
+        output = conn.send_command(cmd) # $ result=BAD getRemoteCommand=cmd
     driver = IOSXEDriver
     with driver(**dev_connect) as conn:
-        output = conn.send_command(cmd) # $ result=BAD getSecondaryCommand=cmd
+        output = conn.send_command(cmd) # $ result=BAD getRemoteCommand=cmd
     driver = EOSDriver
     with driver(**dev_connect) as conn:
-        output = conn.send_command(cmd) # $ result=BAD getSecondaryCommand=cmd
+        output = conn.send_command(cmd) # $ result=BAD getRemoteCommand=cmd
     driver = JunosDriver
     with driver(**dev_connect) as conn:
-        output = conn.send_command(cmd) # $ result=BAD getSecondaryCommand=cmd
+        output = conn.send_command(cmd) # $ result=BAD getRemoteCommand=cmd
 
     dev_connect = {
         "host": "65.65.65.65",
@@ -71,7 +71,7 @@ def bad2(cmd: str):
         "platform": "cisco_iosxe",
     }
     with Scrapli(**dev_connect) as conn:
-        result = conn.send_command(cmd) # $ result=BAD getSecondaryCommand=cmd
+        result = conn.send_command(cmd) # $ result=BAD getRemoteCommand=cmd
 
     dev_connect = {
         "host": "65.65.65.65",
@@ -81,5 +81,5 @@ def bad2(cmd: str):
         "transport": "ssh2",
     }
     with GenericDriver(**dev_connect) as conn:
-        result = conn.send_command(cmd) # $ result=BAD getSecondaryCommand=cmd
+        result = conn.send_command(cmd) # $ result=BAD getRemoteCommand=cmd
     return {"success": "Dangerous"}
diff --git a/python/ql/test/experimental/query-tests/Security/CWE-074-RemoteCommandExecution/Twisted.py b/python/ql/test/experimental/query-tests/Security/CWE-074-RemoteCommandExecution/Twisted.py
@@ -13,15 +13,15 @@
 async def bad1(cmd: bytes):
     endpoint = SSHCommandClientEndpoint.newConnection(
     reactor,
-    cmd, # $ result=BAD getSecondaryCommand=cmd  
+    cmd, # $ result=BAD getRemoteCommand=cmd  
     b"username",
     b"ssh.example.com",
     22,
     password=b"password") 
 
     SSHCommandClientEndpoint.existingConnection(
         endpoint,
-        cmd) # $ result=BAD getSecondaryCommand=cmd
+        cmd) # $ result=BAD getRemoteCommand=cmd
     
     factory = Factory()
     d = endpoint.connect(factory)
diff --git a/python/ql/test/experimental/query-tests/Security/CWE-074-RemoteCommandExecution/paramiko.py b/python/ql/test/experimental/query-tests/Security/CWE-074-RemoteCommandExecution/paramiko.py
@@ -13,10 +13,10 @@
 
 @app.get("/bad1")
 async def bad1(cmd: str):
-    stdin, stdout, stderr = paramiko_ssh_client.exec_command(cmd)  # $ result=BAD getSecondaryCommand=cmd
+    stdin, stdout, stderr = paramiko_ssh_client.exec_command(cmd)  # $ result=BAD getRemoteCommand=cmd
     return {"success": "Dangerous"}
 
 @app.get("/bad2")
 async def bad2(cmd: str):
-    stdin, stdout, stderr = paramiko_ssh_client.exec_command(command=cmd)  # $ result=BAD getSecondaryCommand=cmd
+    stdin, stdout, stderr = paramiko_ssh_client.exec_command(command=cmd)  # $ result=BAD getRemoteCommand=cmd
     return {"success": "Dangerous"}
diff --git a/python/ql/test/experimental/query-tests/Security/CWE-074-RemoteCommandExecution/ssh2.py b/python/ql/test/experimental/query-tests/Security/CWE-074-RemoteCommandExecution/ssh2.py
@@ -14,7 +14,7 @@
 @app.get("/bad1")
 async def bad1(cmd: str):
     channel = session.open_session()
-    channel.execute(cmd) # $ result=BAD getSecondaryCommand=cmd
+    channel.execute(cmd) # $ result=BAD getRemoteCommand=cmd
     channel.wait_eof()
     channel.close()
     channel.wait_closed()  
diff --git a/python/ql/test/experimental/query-tests/Security/CWE-074-SecondaryServerCmdInjection/DataflowQueryTest.ql b/python/ql/test/experimental/query-tests/Security/CWE-074-SecondaryServerCmdInjection/DataflowQueryTest.ql
diff --git a/python/ql/test/experimental/query-tests/Security/CWE-074-SecondaryServerCmdInjection/SecondaryServerCmdInjection.qlref b/python/ql/test/experimental/query-tests/Security/CWE-074-SecondaryServerCmdInjection/SecondaryServerCmdInjection.qlref

Original file line number	Diff line number	Diff line change
`@@ -6,16 +6,16 @@ private import semmle.python.dataflow.new.internal.PrintNode`
`6`	`6`	`import experimental.semmle.python.Concepts`
`7`	`7`
`8`	`8`	`module SystemCommandExecutionTest implements TestSig {`
`9`		`- string getARelevantTag() { result = "getSecondaryCommand" }`
	`9`	`+ string getARelevantTag() { result = "getRemoteCommand" }`
`10`	`10`
`11`	`11`	`predicate hasActualResult(Location location, string element, string tag, string value) {`
`12`	`12`	`exists(location.getFile().getRelativePath()) and`
`13`		`- exists(SecondaryCommandInjection sci, DataFlow::Node command \|`
`14`		`- command = sci and`
	`13`	`+ exists(RemoteCommandExecution sci, DataFlow::Node command \|`
	`14`	`+ command = sci.getCommand() and`
`15`	`15`	`location = command.getLocation() and`
`16`	`16`	`element = command.toString() and`
`17`	`17`	`value = prettyNodeForInlineTest(command) and`
`18`		`- tag = "getSecondaryCommand"`
	`18`	`+ tag = "getRemoteCommand"`
`19`	`19`	`)`
`20`	`20`	`}`
`21`	`21`	`}`