Merge pull request #530 from markshannon/python-no-cert-validation

New query to check for making a request without cert verification.

change-notes/1.19/analysis-python.md

@@ -57,6 +57,7 @@ A new predicate `Stmt.getAnEntryNode()` has been added to make it easier to writ
 | **Query**                   | **Tags**  | **Purpose**                                                        |
 |-----------------------------|-----------|--------------------------------------------------------------------|
 | Information exposure through an exception (`py/stack-trace-exposure`) | security, external/cwe/cwe-209, external/cwe/cwe-497 | Finds instances where information about an exception may be leaked to an external user. Enabled on LGTM by default. |
+| Request without certificate validation (`py/request-without-cert-validation`) | security, external/cwe/cwe-295 | Finds requests where certificate verification has been explicitly turned off, possibly allowing man-in-the-middle attacks. Not enabled on LGTM by default. |
 
 ## Changes to existing queries
 

python/ql/src/Security/CWE-295/RequestWithoutValidation.qhelp

@@ -0,0 +1,36 @@
+<!DOCTYPE qhelp PUBLIC
+  "-//Semmle//qhelp//EN"
+  "qhelp.dtd">
+<qhelp>
+
+<overview>
+<p>
+Encryption is key to the security of most, if not all, online communication.
+Using Transport Layer Security (TLS) can ensure that communication cannot be interrupted by an interloper.
+For this reason, is is unwise to disable the verification that TLS provides.
+Functions in the <code>requests</code> module provide verification by default, and it is only when
+explicitly turned off using <code>verify=False</code> that no verification occurs.
+</p>
+</overview>
+
+<recommendation>
+<p>
+Never use <code>verify=False</code> when making a request.
+</p>
+</recommendation>
+
+<example>
+<p>
+The example shows two unsafe calls to <a href="https://semmle.com">semmle.com</a>, followed by various safe alternatives.
+</p>
+
+<sample src="examples/make_request.py" />
+</example>
+
+<references>
+<li>
+Python requests documentation: <a href="http://docs.python-requests.org/en/master/user/advanced/#ssl-cert-verification">SSL Cert Verification</a>.
+</li>
+</references>
+</qhelp>
+

python/ql/src/Security/CWE-295/RequestWithoutValidation.ql

@@ -0,0 +1,36 @@
+/**
+ * @name Request without certificate validation
+ * @description Making a request without certificate validation can allow man-in-the-middle attacks.
+ * @kind problem
+ * @problem.severity error
+ * @precision medium
+ * @id py/request-without-cert-validation
+ * @tags security
+ *       external/cwe/cwe-295
+ */
+
+import python
+
+import semmle.python.web.Http
+
+
+FunctionObject requestFunction() {
+    exists(ModuleObject req |
+        req.getName() = "requests" and
+        result = req.getAttribute(httpVerbLower())
+    )
+}
+
+/** requests treats None as the default and all other "falsey" values as False */
+predicate falseNotNone(Object o) {
+    o.booleanValue() = false and not o = theNoneObject()
+}
+
+from CallNode call, FunctionObject func, Object falsey, ControlFlowNode origin
+where 
+func = requestFunction() and
+func.getACall() = call and
+falseNotNone(falsey) and
+call.getArgByName("verify").refersTo(falsey, origin)
+
+select call, "Call to $@ with verify=$@", func, "requests." + func.getName(), origin, "False"

python/ql/src/Security/CWE-295/examples/make_request.py

@@ -0,0 +1,19 @@
+import requests
+
+#Unsafe requests
+
+requests.get('https://semmle.com', verify=False) # UNSAFE
+requests.get('https://semmle.com', verify=0) # UNSAFE
+
+#Various safe options
+
+requests.get('https://semmle.com', verify=True) # Explicitly safe
+requests.get('https://semmle.com', verify="/path/to/cert/")
+requests.get('https://semmle.com') # The default is to verify.
+
+#Wrapper to ensure safety
+
+def make_safe_request(url, verify_cert):
+    if not verify_cert:
+        raise Exception("Trying to make unsafe request")
+    return requests.get(url, verify_cert)

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

@@ -0,0 +1,5 @@
+| make_request.py:5:1:5:48 | ControlFlowNode for Attribute() | Call to $@ with verify=$@ | ../lib/requests.py:2:1:2:36 | Function get | requests.get | make_request.py:5:43:5:47 | ControlFlowNode for False | False |
+| make_request.py:7:1:7:49 | ControlFlowNode for Attribute() | Call to $@ with verify=$@ | ../lib/requests.py:11:1:11:46 | Function post | requests.post | make_request.py:7:44:7:48 | ControlFlowNode for False | False |
+| make_request.py:12:1:12:39 | ControlFlowNode for put() | Call to $@ with verify=$@ | ../lib/requests.py:14:1:14:34 | Function put | requests.put | make_request.py:12:34:12:38 | ControlFlowNode for False | False |
+| make_request.py:28:5:28:46 | ControlFlowNode for patch() | Call to $@ with verify=$@ | ../lib/requests.py:17:1:17:36 | Function patch | requests.patch | make_request.py:30:6:30:10 | ControlFlowNode for False | False |
+| make_request.py:34:1:34:45 | ControlFlowNode for Attribute() | Call to $@ with verify=$@ | ../lib/requests.py:11:1:11:46 | Function post | requests.post | make_request.py:34:44:34:44 | ControlFlowNode for IntegerLiteral | False |

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

@@ -0,0 +1 @@
+Security/CWE-295/RequestWithoutValidation.ql

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

@@ -0,0 +1,34 @@
+import requests
+
+#Simple cases
+requests.get('https://semmle.com', verify=True) # GOOD
+requests.get('https://semmle.com', verify=False) # BAD
+requests.post('https://semmle.com', verify=True) # GOOD
+requests.post('https://semmle.com', verify=False) # BAD
+
+# Simple flow
+put = requests.put
+put('https://semmle.com', verify="/path/to/cert/") # GOOD
+put('https://semmle.com', verify=False) # BAD
+
+#Other flow
+delete = requests.delete
+
+def req1(verify=False):
+    delete('https://semmle.com', verify) # BAD
+    if verify:
+        delete('https://semmle.com', verify) # GOOD
+    if not verify:
+        return
+    delete('https://semmle.com', verify) # GOOD
+
+patch = requests.patch
+
+def req2(verify):
+    patch('https://semmle.com', verify=verify) # BAD (from line 30)
+
+req2(False) # BAD (at line 28)
+req2("/path/to/cert/") # GOOD
+
+#Falsey value
+requests.post('https://semmle.com', verify=0) # BAD

python/ql/test/query-tests/Security/CWE-295/options

@@ -0,0 +1 @@
+semmle-extractor-options: -p ../lib/ --max-import-depth=3

python/ql/test/query-tests/Security/lib/requests.py

@@ -0,0 +1,21 @@
+
+def get(url, params=None, **kwargs):
+    pass
+
+def options(url, **kwargs):
+    pass
+
+def head(url, **kwargs):
+    pass
+
+def post(url, data=None, json=None, **kwargs):
+    pass
+
+def put(url, data=None, **kwargs):
+    pass
+
+def patch(url, data=None, **kwargs):
+    pass
+
+def delete(url, **kwargs):
+    pass