Merge pull request #785 from taus-semmle/python-unsafe-use-of-mktemp

Python: Add query for unsafe use of `tempfile.mktemp`.

Author: markshannon

change-notes/1.20/analysis-python.md

@@ -19,6 +19,7 @@ The API has been improved to declutter the global namespace and improve discover
 | Default version of SSL/TLS may be insecure (`py/insecure-default-protocol`) | security, external/cwe/cwe-327 | Finds instances where an insecure default protocol may be used. Results are shown on LGTM by default. |
 | Incomplete regular expression for hostnames (`py/incomplete-hostname-regexp`) | security, external/cwe/cwe-020 | Finds instances where a hostname is incompletely sanitized because a regular expression contains an unescaped character. Results are shown on LGTM by default. |
 | Incomplete URL substring sanitization (`py/incomplete-url-substring-sanitization`) | security, external/cwe/cwe-020 | Finds instances where a URL is incompletely sanitized due to insufficient checks. Results are shown on LGTM by default. |
+| Insecure temporary file (`py/insecure-temporary-file`) | security, external/cwe/cwe-377 | Finds uses of the insecure and deprecated `tempfile.mktemp`, `os.tempnam`, and `os.tmpnam` functions. Results are shown on LGTM by default. |
 | Overly permissive file permissions (`py/overly-permissive-file`) | security, external/cwe/cwe-732 | Finds instances where a file is created with overly permissive permissions. Results are not shown on LGTM by default. |
 | Use of insecure SSL/TLS version (`py/insecure-protocol`) | security, external/cwe/cwe-327 | Finds instances where a known insecure protocol has been specified. Results are shown on LGTM by default. |
 

python/ql/src/Security/CWE-377/InsecureTemporaryFile.py

@@ -0,0 +1,7 @@
+from tempfile import mktemp
+
+def write_results(results):
+    filename = mktemp()
+    with open(filename, "w+") as f:
+        f.write(results)
+    print("Results written to", filename)

python/ql/src/Security/CWE-377/InsecureTemporaryFile.qhelp

@@ -0,0 +1,52 @@
+<!DOCTYPE qhelp PUBLIC "-//Semmle//qhelp//EN" "qhelp.dtd">
+<qhelp>
+
+<overview>
+<p>
+Functions that create temporary file names (such as <code>tempfile.mktemp</code>
+and <code>os.tempnam</code>) are fundamentally insecure, as they do not
+ensure exclusive access to a file with the temporary name they return.
+The file name returned by these functions is guaranteed to be unique on
+creation but the file must be opened in a separate operation. There is no
+guarantee that the creation and open operations will happen atomically. This
+provides an opportunity for an attacker to interfere with the file before it is
+opened.
+</p>
+<p>
+Note that <code>mktemp</code> has been deprecated since Python 2.3.
+</p>
+</overview>
+
+<recommendation>
+<p>
+Replace the use of <code>mktemp</code> with some of the more secure functions
+in the <code>tempfile</code> module, such as <code>TemporaryFile</code>. If the
+file is intended to be accessed from other processes, consider using the
+<code>NamedTemporaryFile</code> function.
+</p>
+</recommendation>
+
+<example>
+<p>
+The following piece of code opens a temporary file and writes a set of results
+to it. Because the file name is created using <code>mktemp</code>, another
+process may access this file before it is opened using <code>open</code>.
+</p>
+<sample src="InsecureTemporaryFile.py" />
+
+<p>
+By changing the code to use <code>NamedTemporaryFile</code> instead, the file is
+opened immediately.
+</p>
+<sample src="SecureTemporaryFile.py" />
+
+</example>
+
+<references>
+
+<li>
+Python Standard Library: <a href="https://docs.python.org/3/library/tempfile.html#tempfile.mktemp">tempfile.mktemp</a>.
+</li>
+</references>
+
+</qhelp>

python/ql/src/Security/CWE-377/InsecureTemporaryFile.ql

@@ -0,0 +1,31 @@
+/**
+ * @name Insecure temporary file
+ * @description Creating a temporary file using this method may be insecure.
+ * @id py/insecure-temporary-file
+ * @problem.severity error
+ * @sub-severity high
+ * @precision high
+ * @tags external/cwe/cwe-377
+ *       security
+ */
+
+import python
+
+FunctionObject temporary_name_function(string mod, string function) {
+    (
+        mod = "tempfile" and function = "mktemp"
+        or
+        mod = "os" and
+        (
+            function = "tmpnam"
+            or
+            function = "tempnam"
+        )
+    ) and
+    result = any(ModuleObject m | m.getName() = mod).getAttribute(function)
+}
+
+from Call c, string mod, string function
+where
+    temporary_name_function(mod, function).getACall().getNode() = c
+select c, "Call to deprecated function " + mod + "." + function + " may be insecure."

python/ql/src/Security/CWE-377/SecureTemporaryFile.py

@@ -0,0 +1,6 @@
+from tempfile import NamedTemporaryFile
+
+def write_results(results):
+    with NamedTemporaryFile(mode="w+", delete=False) as f:
+        f.write(results)
+    print("Results written to", f.name)

python/ql/test/query-tests/Security/CWE-377/InsecureTemporaryFile.expected

@@ -0,0 +1,3 @@
+| InsecureTemporaryFile.py:5:16:5:23 | mktemp() | Call to deprecated function tempfile.mktemp may be insecure. |
+| InsecureTemporaryFile.py:11:16:11:27 | Attribute() | Call to deprecated function os.tempnam may be insecure. |
+| InsecureTemporaryFile.py:17:16:17:26 | Attribute() | Call to deprecated function os.tmpnam may be insecure. |

python/ql/test/query-tests/Security/CWE-377/InsecureTemporaryFile.py

@@ -0,0 +1,20 @@
+from tempfile import mktemp
+import os
+
+def write_results1(results):
+    filename = mktemp()
+    with open(filename, "w+") as f:
+        f.write(results)
+    print("Results written to", filename)
+
+def write_results2(results):
+    filename = os.tempnam()
+    with open(filename, "w+") as f:
+        f.write(results)
+    print("Results written to", filename)
+
+def write_results3(results):
+    filename = os.tmpnam()
+    with open(filename, "w+") as f:
+        f.write(results)
+    print("Results written to", filename)

python/ql/test/query-tests/Security/CWE-377/InsecureTemporaryFile.qlref

@@ -0,0 +1 @@
+Security/CWE-377/InsecureTemporaryFile.ql

python/ql/test/query-tests/Security/CWE-377/SecureTemporaryFile.py

@@ -0,0 +1,6 @@
+from tempfile import NamedTemporaryFile
+
+def write_results(results):
+    with NamedTemporaryFile(mode="w+", delete=False) as f:
+        f.write(results)
+    print("Results written to", f.name)

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

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