FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

Merge pull request #1128 from taus-semmle/python-paramiko-unsafe-host… · github/codeql@2ba1223 · GitHub

/ codeql Public

Commit 2ba1223

Browse files
markshannon
authored
Merge pull request #1128 from taus-semmle/python-paramiko-unsafe-host-key-validation
Python: Add query for insecure SSH host key policies in Paramiko.
2 parents c5f41c1 + 20e2f9e commit 2ba1223

9 files changed

Lines changed: 157 additions & 0 deletions

File tree

‎change-notes/1.21/analysis-python.md‎

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
# Improvements to Python analysis
2+
3+
4+
## General improvements
5+
6+
> Changes that affect alerts in many files or from many queries
7+
> For example, changes to file classification
8+
9+
## New queries
10+
| **Query** | **Tags** | **Purpose** |
11+
|-----------|----------|-------------|
12+
| 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. |
13+
14+
15+
## Changes to existing queries
16+
17+
| **Query** | **Expected impact** | **Change** |
18+
|-----------|---------------------|------------|
19+
20+
## Changes to code extraction
21+
22+
* *Series of bullet points*
23+
24+
## Changes to QL libraries
25+
26+
* *Series of bullet points*

‎python/ql/src/Security/CWE-295/MissingHostKeyValidation.qhelp‎

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,43 @@
1+
<!DOCTYPE qhelp PUBLIC
2+
"-//Semmle//qhelp//EN"
3+
"qhelp.dtd">
4+
<qhelp>
5+
6+
<overview>
7+
<p>
8+
In the Secure Shell (SSH) protocol, host keys are used to verify the identity of
9+
remote hosts. Accepting unknown host keys may leave the connection open to
10+
man-in-the-middle attacks.
11+
</p>
12+
</overview>
13+
14+
<recommendation>
15+
<p>
16+
Do not accept unknown host keys. In particular, do not set the default missing
17+
host key policy for the Paramiko library to either <code>AutoAddPolicy</code> or
18+
<code>WarningPolicy</code>. Both of these policies continue even when the host
19+
key is unknown. The default setting of <code>RejectPolicy</code> is secure
20+
because it throws an exception when it encounters an unknown host key.
21+
</p>
22+
</recommendation>
23+
24+
<example>
25+
<p>
26+
The following example shows two ways of opening an SSH connection to
27+
<code>example.com</code>. The first function sets the missing host key policy to
28+
<code>AutoAddPolicy</code>. If the host key verification fails, the client will
29+
continue to interact with the server, even though the connection may be
30+
compromised. The second function sets the host key policy to
31+
<code>RejectPolicy</code>, and will throw an exception if the host key
32+
verification fails.
33+
</p>
34+
<sample src="examples/paramiko_host_key.py" />
35+
</example>
36+
37+
<references>
38+
<li>
39+
Paramiko documentation: <a href="http://docs.paramiko.org/en/2.4/api/client.html?highlight=set_missing_host_key_policy#paramiko.client.SSHClient.set_missing_host_key_policy">set_missing_host_key_policy</a>.
40+
</li>
41+
</references>
42+
</qhelp>
43+

‎python/ql/src/Security/CWE-295/MissingHostKeyValidation.ql‎

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,36 @@
1+
/**
2+
* @name Accepting unknown SSH host keys when using Paramiko
3+
* @description Accepting unknown host keys can allow man-in-the-middle attacks.
4+
* @kind problem
5+
* @problem.severity error
6+
* @precision high
7+
* @id py/paramiko-missing-host-key-validation
8+
* @tags security
9+
* external/cwe/cwe-295
10+
*/
11+
12+
import python
13+
14+
private ModuleObject theParamikoClientModule() { result = ModuleObject::named("paramiko.client") }
15+
16+
private ClassObject theParamikoSSHClientClass() {
17+
result = theParamikoClientModule().attr("SSHClient")
18+
}
19+
20+
private ClassObject unsafe_paramiko_policy(string name) {
21+
(name = "AutoAddPolicy" or name = "WarningPolicy") and
22+
result = theParamikoClientModule().attr(name)
23+
}
24+
25+
from CallNode call, ControlFlowNode arg, string name
26+
where
27+
call = theParamikoSSHClientClass()
28+
.lookupAttribute("set_missing_host_key_policy")
29+
.(FunctionObject)
30+
.getACall() and
31+
arg = call.getAnArg() and
32+
(
33+
arg.refersTo(unsafe_paramiko_policy(name)) or
34+
arg.refersTo(_, unsafe_paramiko_policy(name), _)
35+
)
36+
select call, "Setting missing host key policy to " + name + " may be unsafe."

‎python/ql/src/Security/CWE-295/examples/paramiko_host_key.py‎

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
from paramiko.client import SSHClient, AutoAddPolicy, RejectPolicy
2+
3+
def unsafe_connect():
4+
client = SSHClient()
5+
client.set_missing_host_key_policy(AutoAddPolicy)
6+
client.connect("example.com")
7+
8+
# ... interaction with server
9+
10+
client.close()
11+
12+
def safe_connect():
13+
client = SSHClient()
14+
client.set_missing_host_key_policy(RejectPolicy)
15+
client.connect("example.com")
16+
17+
# ... interaction with server
18+
19+
client.close()

‎python/ql/test/query-tests/Security/CWE-295/MissingHostKeyValidation.expected‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
| paramiko_host_key.py:5:1:5:49 | ControlFlowNode for Attribute() | Setting missing host key policy to AutoAddPolicy may be unsafe. |
2+
| paramiko_host_key.py:7:1:7:49 | ControlFlowNode for Attribute() | Setting missing host key policy to WarningPolicy may be unsafe. |
3+
| paramiko_host_key.py:11:1:11:51 | ControlFlowNode for Attribute() | Setting missing host key policy to AutoAddPolicy may be unsafe. |
4+
| paramiko_host_key.py:13:1:13:51 | ControlFlowNode for Attribute() | Setting missing host key policy to WarningPolicy may be unsafe. |

‎python/ql/test/query-tests/Security/CWE-295/MissingHostKeyValidation.qlref‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
Security/CWE-295/MissingHostKeyValidation.ql

‎python/ql/test/query-tests/Security/CWE-295/paramiko_host_key.py‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
from paramiko.client import AutoAddPolicy, WarningPolicy, RejectPolicy, SSHClient
2+
3+
client = SSHClient()
4+
5+
client.set_missing_host_key_policy(AutoAddPolicy) # bad
6+
client.set_missing_host_key_policy(RejectPolicy) # good
7+
client.set_missing_host_key_policy(WarningPolicy) # bad
8+
9+
# Using instances
10+
11+
client.set_missing_host_key_policy(AutoAddPolicy()) # bad
12+
client.set_missing_host_key_policy(RejectPolicy()) # good
13+
client.set_missing_host_key_policy(WarningPolicy()) # bad

‎python/ql/test/query-tests/Security/lib/paramiko/__init__.py‎

Whitespace-only changes.

‎python/ql/test/query-tests/Security/lib/paramiko/client.py‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
class SSHClient(object):
2+
def __init__(self, *args, **kwargs):
3+
pass
4+
5+
def set_missing_host_key_policy(self, *args, **kwargs):
6+
pass
7+
8+
class AutoAddPolicy(object):
9+
pass
10+
11+
class WarningPolicy(object):
12+
pass
13+
14+
class RejectPolicy(object):
15+
pass

0 commit comments

Comments
 (0)
Back | FazBrowse Home | New Git URL