diff --git a/change-notes/1.21/analysis-python.md b/change-notes/1.21/analysis-python.md new file mode 100644 index 000000000000..49f0127c6043 --- /dev/null +++ b/change-notes/1.21/analysis-python.md @@ -0,0 +1,26 @@ +# Improvements to Python analysis + + +## General improvements + +> Changes that affect alerts in many files or from many queries +> For example, changes to file classification + +## New queries + | **Query** | **Tags** | **Purpose** | + |-----------|----------|-------------| + | Accepting unknown SSH host keys when using Paramiko (`py/paramiko-missing-host-key-validation`) | security, external/cwe/cwe-295 | Finds instances where Paramiko is configured to accept unknown host keys. Results are shown on LGTM by default. | + + +## Changes to existing queries + + | **Query** | **Expected impact** | **Change** | + |-----------|---------------------|------------| + +## Changes to code extraction + +* *Series of bullet points* + +## Changes to QL libraries + +* *Series of bullet points* diff --git a/python/ql/src/Security/CWE-295/MissingHostKeyValidation.qhelp b/python/ql/src/Security/CWE-295/MissingHostKeyValidation.qhelp new file mode 100644 index 000000000000..a32debbac427 --- /dev/null +++ b/python/ql/src/Security/CWE-295/MissingHostKeyValidation.qhelp @@ -0,0 +1,43 @@ + + + + +

+In the Secure Shell (SSH) protocol, host keys are used to verify the identity of +remote hosts. Accepting unknown host keys may leave the connection open to +man-in-the-middle attacks. +

+
+ + +

+Do not accept unknown host keys. In particular, do not set the default missing +host key policy for the Paramiko library to either AutoAddPolicy or +WarningPolicy. Both of these policies continue even when the host +key is unknown. The default setting of RejectPolicy is secure +because it throws an exception when it encounters an unknown host key. +

+
+ + +

+The following example shows two ways of opening an SSH connection to +example.com. The first function sets the missing host key policy to +AutoAddPolicy. If the host key verification fails, the client will +continue to interact with the server, even though the connection may be +compromised. The second function sets the host key policy to +RejectPolicy, and will throw an exception if the host key +verification fails. +

+ +
+ + +
  • +Paramiko documentation: set_missing_host_key_policy. +
  • +
    +
    + diff --git a/python/ql/src/Security/CWE-295/MissingHostKeyValidation.ql b/python/ql/src/Security/CWE-295/MissingHostKeyValidation.ql new file mode 100644 index 000000000000..5ebca56986f0 --- /dev/null +++ b/python/ql/src/Security/CWE-295/MissingHostKeyValidation.ql @@ -0,0 +1,36 @@ +/** + * @name Accepting unknown SSH host keys when using Paramiko + * @description Accepting unknown host keys can allow man-in-the-middle attacks. + * @kind problem + * @problem.severity error + * @precision high + * @id py/paramiko-missing-host-key-validation + * @tags security + * external/cwe/cwe-295 + */ + +import python + +private ModuleObject theParamikoClientModule() { result = ModuleObject::named("paramiko.client") } + +private ClassObject theParamikoSSHClientClass() { + result = theParamikoClientModule().attr("SSHClient") +} + +private ClassObject unsafe_paramiko_policy(string name) { + (name = "AutoAddPolicy" or name = "WarningPolicy") and + result = theParamikoClientModule().attr(name) +} + +from CallNode call, ControlFlowNode arg, string name +where + call = theParamikoSSHClientClass() + .lookupAttribute("set_missing_host_key_policy") + .(FunctionObject) + .getACall() and + arg = call.getAnArg() and + ( + arg.refersTo(unsafe_paramiko_policy(name)) or + arg.refersTo(_, unsafe_paramiko_policy(name), _) + ) +select call, "Setting missing host key policy to " + name + " may be unsafe." diff --git a/python/ql/src/Security/CWE-295/examples/paramiko_host_key.py b/python/ql/src/Security/CWE-295/examples/paramiko_host_key.py new file mode 100644 index 000000000000..d197cb9741dd --- /dev/null +++ b/python/ql/src/Security/CWE-295/examples/paramiko_host_key.py @@ -0,0 +1,19 @@ +from paramiko.client import SSHClient, AutoAddPolicy, RejectPolicy + +def unsafe_connect(): + client = SSHClient() + client.set_missing_host_key_policy(AutoAddPolicy) + client.connect("example.com") + + # ... interaction with server + + client.close() + +def safe_connect(): + client = SSHClient() + client.set_missing_host_key_policy(RejectPolicy) + client.connect("example.com") + + # ... interaction with server + + client.close() diff --git a/python/ql/test/query-tests/Security/CWE-295/MissingHostKeyValidation.expected b/python/ql/test/query-tests/Security/CWE-295/MissingHostKeyValidation.expected new file mode 100644 index 000000000000..bcbb79ad6ff4 --- /dev/null +++ b/python/ql/test/query-tests/Security/CWE-295/MissingHostKeyValidation.expected @@ -0,0 +1,4 @@ +| paramiko_host_key.py:5:1:5:49 | ControlFlowNode for Attribute() | Setting missing host key policy to AutoAddPolicy may be unsafe. | +| paramiko_host_key.py:7:1:7:49 | ControlFlowNode for Attribute() | Setting missing host key policy to WarningPolicy may be unsafe. | +| paramiko_host_key.py:11:1:11:51 | ControlFlowNode for Attribute() | Setting missing host key policy to AutoAddPolicy may be unsafe. | +| paramiko_host_key.py:13:1:13:51 | ControlFlowNode for Attribute() | Setting missing host key policy to WarningPolicy may be unsafe. | diff --git a/python/ql/test/query-tests/Security/CWE-295/MissingHostKeyValidation.qlref b/python/ql/test/query-tests/Security/CWE-295/MissingHostKeyValidation.qlref new file mode 100644 index 000000000000..c366095516af --- /dev/null +++ b/python/ql/test/query-tests/Security/CWE-295/MissingHostKeyValidation.qlref @@ -0,0 +1 @@ +Security/CWE-295/MissingHostKeyValidation.ql diff --git a/python/ql/test/query-tests/Security/CWE-295/paramiko_host_key.py b/python/ql/test/query-tests/Security/CWE-295/paramiko_host_key.py new file mode 100644 index 000000000000..2dd13aafe18a --- /dev/null +++ b/python/ql/test/query-tests/Security/CWE-295/paramiko_host_key.py @@ -0,0 +1,13 @@ +from paramiko.client import AutoAddPolicy, WarningPolicy, RejectPolicy, SSHClient + +client = SSHClient() + +client.set_missing_host_key_policy(AutoAddPolicy) # bad +client.set_missing_host_key_policy(RejectPolicy) # good +client.set_missing_host_key_policy(WarningPolicy) # bad + +# Using instances + +client.set_missing_host_key_policy(AutoAddPolicy()) # bad +client.set_missing_host_key_policy(RejectPolicy()) # good +client.set_missing_host_key_policy(WarningPolicy()) # bad diff --git a/python/ql/test/query-tests/Security/lib/paramiko/__init__.py b/python/ql/test/query-tests/Security/lib/paramiko/__init__.py new file mode 100644 index 000000000000..e69de29bb2d1 diff --git a/python/ql/test/query-tests/Security/lib/paramiko/client.py b/python/ql/test/query-tests/Security/lib/paramiko/client.py new file mode 100644 index 000000000000..6343fbe78dd3 --- /dev/null +++ b/python/ql/test/query-tests/Security/lib/paramiko/client.py @@ -0,0 +1,15 @@ +class SSHClient(object): + def __init__(self, *args, **kwargs): + pass + + def set_missing_host_key_policy(self, *args, **kwargs): + pass + +class AutoAddPolicy(object): + pass + +class WarningPolicy(object): + pass + +class RejectPolicy(object): + pass