From 7dd1389b9e5f2b883cd9a0e3118a2e55374e8f6e Mon Sep 17 00:00:00 2001
From: amammad <77095239+amammad@users.noreply.github.com>
Date: Sun, 25 Feb 2024 17:52:24 +0400
Subject: [PATCH] add twisted SSH client as secondary server command injection
 sinks, add proper test cases

---
 .../experimental/semmle/python/Frameworks.qll |  1 +
 .../semmle/python/frameworks/Twisted.qll      | 36 +++++++++++++++++++
 .../Twisted.py                                | 30 ++++++++++++++++
 .../paramiko.py                               |  4 +--
 4 files changed, 69 insertions(+), 2 deletions(-)
 create mode 100644 python/ql/src/experimental/semmle/python/frameworks/Twisted.qll
 create mode 100644 python/ql/test/experimental/query-tests/Security/CWE-074-SecondaryServerCmdInjection/Twisted.py

diff --git a/python/ql/src/experimental/semmle/python/Frameworks.qll b/python/ql/src/experimental/semmle/python/Frameworks.qll
index 1d8fe10e911..6e6d4f25317 100644
--- a/python/ql/src/experimental/semmle/python/Frameworks.qll
+++ b/python/ql/src/experimental/semmle/python/Frameworks.qll
@@ -12,6 +12,7 @@ private import experimental.semmle.python.frameworks.Netmiko
 private import experimental.semmle.python.frameworks.Paramiko
 private import experimental.semmle.python.frameworks.Pexpect
 private import experimental.semmle.python.frameworks.Scrapli
+private import experimental.semmle.python.frameworks.Twisted
 private import experimental.semmle.python.frameworks.JWT
 private import experimental.semmle.python.frameworks.Csv
 private import experimental.semmle.python.libraries.PyJWT
diff --git a/python/ql/src/experimental/semmle/python/frameworks/Twisted.qll b/python/ql/src/experimental/semmle/python/frameworks/Twisted.qll
new file mode 100644
index 00000000000..ad7de833473
--- /dev/null
+++ b/python/ql/src/experimental/semmle/python/frameworks/Twisted.qll
@@ -0,0 +1,36 @@
+/**
+ * Provides classes modeling security-relevant aspects of the `twisted` PyPI package.
+ * See https://twistedmatrix.com/.
+ */
+
+private import python
+private import semmle.python.dataflow.new.DataFlow
+private import semmle.python.dataflow.new.RemoteFlowSources
+private import semmle.python.dataflow.new.TaintTracking
+private import semmle.python.Concepts
+private import semmle.python.ApiGraphs
+private import semmle.python.frameworks.internal.InstanceTaintStepsHelper
+import experimental.semmle.python.Concepts
+
+/**
+ * Provides models for the `twisted` PyPI package.
+ * See https://twistedmatrix.com/.
+ */
+private module Twisted {
+  /**
+   * The `newConnection` and `existingConnection` functions of `twisted.conch.endpoints.SSHCommandClientEndpoint` class execute command on ssh target server
+   */
+  class ParamikoExecCommand extends SecondaryCommandInjection {
+    ParamikoExecCommand() {
+      this =
+        API::moduleImport("twisted")
+            .getMember("conch")
+            .getMember("endpoints")
+            .getMember("SSHCommandClientEndpoint")
+            .getMember(["newConnection", "existingConnection"])
+            .getACall()
+            .getParameter(1, "command")
+            .asSink()
+    }
+  }
+}
diff --git a/python/ql/test/experimental/query-tests/Security/CWE-074-SecondaryServerCmdInjection/Twisted.py b/python/ql/test/experimental/query-tests/Security/CWE-074-SecondaryServerCmdInjection/Twisted.py
new file mode 100644
index 00000000000..e432f37a126
--- /dev/null
+++ b/python/ql/test/experimental/query-tests/Security/CWE-074-SecondaryServerCmdInjection/Twisted.py
@@ -0,0 +1,30 @@
+#!/usr/bin/env python
+
+from fastapi import FastAPI
+from twisted.conch.endpoints import SSHCommandClientEndpoint
+from twisted.internet.protocol import Factory
+from twisted.internet import reactor
+
+
+app = FastAPI()
+
+
+@app.get("/bad1")
+async def bad1(cmd: bytes):
+    endpoint = SSHCommandClientEndpoint.newConnection(
+    reactor,
+    cmd, # $ result=BAD getSecondaryCommand=cmd  
+    b"username",
+    b"ssh.example.com",
+    22,
+    password=b"password") 
+
+    SSHCommandClientEndpoint.existingConnection(
+        endpoint,
+        cmd) # $ result=BAD getSecondaryCommand=cmd
+    
+    factory = Factory()
+    d = endpoint.connect(factory)
+    d.addCallback(lambda protocol: protocol.finished)
+
+    return {"success": "Dangerous"}
diff --git a/python/ql/test/experimental/query-tests/Security/CWE-074-SecondaryServerCmdInjection/paramiko.py b/python/ql/test/experimental/query-tests/Security/CWE-074-SecondaryServerCmdInjection/paramiko.py
index cf2d0d5970d..96adfdd9946 100644
--- a/python/ql/test/experimental/query-tests/Security/CWE-074-SecondaryServerCmdInjection/paramiko.py
+++ b/python/ql/test/experimental/query-tests/Security/CWE-074-SecondaryServerCmdInjection/paramiko.py
@@ -14,9 +14,9 @@ app = FastAPI()
 @app.get("/bad1")
 async def bad1(cmd: str):
     stdin, stdout, stderr = paramiko_ssh_client.exec_command(cmd)  # $ result=BAD getSecondaryCommand=cmd
-    return {"success": stdout}
+    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
-    return {"success": "OK"}
+    return {"success": "Dangerous"}