Replace Python's deprecated tempfile.mktemp() with secure temp APIs#210123
Merged
Conversation
…temp APIs tempfile.mktemp() is deprecated and insecure: it returns a path without creating the file, leaving a TOCTOU/symlink window before the file is opened. Replace every use, choosing the temp API by lifetime: - compiler-rt android_common.py adb(): tempfile.TemporaryFile (anonymous, auto-deleted), read back via seek(0); drops manual close()/unlink(). - compiler-rt android_common.py pull_from_device(): tempfile.TemporaryDirectory so "adb pull" writes into a private, auto-cleaned directory. - lldb examples delta.py / gdbremote.py start_gdb_log(): tempfile.NamedTemporaryFile(delete=False); the log must outlive the call for a later stop_gdb_log, so it is intentionally not auto-deleted. Reported by internal static analysis and contributed upstream for the benefit of the wider community. Assisted-by: GitHub Copilot CLI (Claude Opus 4.8) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 0261dfbe-a4a8-4868-a4a8-2ca5b15c33e8
|
@llvm/pr-subscribers-lldb @llvm/pr-subscribers-compiler-rt-sanitizer Author: Zachary Henkel (ZacharyHenkel) ChangesPython's tempfile.mktemp() is deprecated and insecure: it returns a path without creating the file, leaving a TOCTOU/symlink window before the file is opened. Replace every use, choosing the temp API by lifetime:
Reported by internal static analysis and contributed upstream for the benefit of the wider community. Assisted-by: GitHub Copilot CLI (Claude Opus 4.8) I do not have commit access and will need someone to submit on my behalf. Full diff: https://github.com/llvm/llvm-project/pull/210123.diff 3 Files Affected:
diff --git a/compiler-rt/test/sanitizer_common/android_commands/android_common.py b/compiler-rt/test/sanitizer_common/android_commands/android_common.py
index 4e7d15c9ebc23..6c7c410108a74 100644
--- a/compiler-rt/test/sanitizer_common/android_commands/android_common.py
+++ b/compiler-rt/test/sanitizer_common/android_commands/android_common.py
@@ -18,33 +18,28 @@ def host_to_device_path(path):
def adb(args, attempts=1, timeout_sec=600):
if verbose:
print(args)
- tmpname = tempfile.mktemp()
- out = open(tmpname, "w")
- ret = 255
- while attempts > 0 and ret != 0:
- attempts -= 1
- ret = subprocess.call(
- ["timeout", str(timeout_sec), ADB] + args,
- stdout=out,
- stderr=subprocess.STDOUT,
- )
- if ret != 0:
- print("adb command failed", args)
- print(tmpname)
- out.close()
- out = open(tmpname, "r")
- print(out.read())
- out.close()
- os.unlink(tmpname)
+ with tempfile.TemporaryFile(mode="w+") as out:
+ ret = 255
+ while attempts > 0 and ret != 0:
+ attempts -= 1
+ ret = subprocess.call(
+ ["timeout", str(timeout_sec), ADB] + args,
+ stdout=out,
+ stderr=subprocess.STDOUT,
+ )
+ if ret != 0:
+ print("adb command failed", args)
+ out.seek(0)
+ print(out.read())
return ret
def pull_from_device(path):
- tmp = tempfile.mktemp()
- adb(["pull", path, tmp], 5, 60)
- text = open(tmp, "r").read()
- os.unlink(tmp)
- return text
+ with tempfile.TemporaryDirectory() as tmp_dir:
+ tmp = os.path.join(tmp_dir, "pulled")
+ adb(["pull", path, tmp], 5, 60)
+ with open(tmp, "r") as f:
+ return f.read()
def push_to_device(path):
diff --git a/lldb/examples/python/delta.py b/lldb/examples/python/delta.py
old mode 100755
new mode 100644
index 35f155bdea100..e84185b1ae05a
--- a/lldb/examples/python/delta.py
+++ b/lldb/examples/python/delta.py
@@ -35,7 +35,8 @@ def start_gdb_log(debugger, command, result, dict):
else:
args_len = len(args)
if args_len == 0:
- log_file = tempfile.mktemp()
+ with tempfile.NamedTemporaryFile(delete=False) as tmp:
+ log_file = tmp.name
elif len(args) == 1:
log_file = args[0]
diff --git a/lldb/examples/python/gdbremote.py b/lldb/examples/python/gdbremote.py
old mode 100755
new mode 100644
index 2f2d82c3d3d54..a215a878ef0a0
--- a/lldb/examples/python/gdbremote.py
+++ b/lldb/examples/python/gdbremote.py
@@ -229,7 +229,8 @@ def start_gdb_log(debugger, command, result, dict):
else:
args_len = len(args)
if args_len == 0:
- g_log_file = tempfile.mktemp()
+ with tempfile.NamedTemporaryFile(delete=False) as tmp:
+ g_log_file = tmp.name
elif len(args) == 1:
g_log_file = args[0]
|
vitalybuka
reviewed
Jul 16, 2026
vitalybuka
reviewed
Jul 16, 2026
| args_len = len(args) | ||
| if args_len == 0: | ||
| log_file = tempfile.mktemp() | ||
| with tempfile.NamedTemporaryFile(delete=False) as tmp: |
Contributor
There was a problem hiding this comment.
lldb and compiler-rt could be 2 independent PRs
Contributor
Author
There was a problem hiding this comment.
These are the only two usages of mktemp in the whole llvm tree so bundling the fixes felt appropriate. If required, I can split the changes into 2 PRs.
Contributor
There was a problem hiding this comment.
If one of this cases is incorrect, unlikely it will apply to the rest of the patch. So I slightly prefer to keep them separate for convenience of revert. And I see no benefits of keeping them together.
🐧 Linux x64 Test Results
✅ The build succeeded and all tests passed. |
Contributor
Could this be related? |
Contributor
|
No it's not related - #193787. |
DavidSpickett
approved these changes
Jul 17, 2026
DavidSpickett
left a comment
DavidSpickett
left a comment
Contributor
There was a problem hiding this comment.
LLDB parts look correct to me. Putting them in their own PR is ideal but I'm not bothered too much because the examples are not critical to lldb.
Ping me on the other PR if you do make one.
vitalybuka
approved these changes
Jul 17, 2026
Contributor
|
CI looks green, so merging this time without splitting. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Python's tempfile.mktemp() is deprecated and insecure: it returns a path without creating the file, leaving a TOCTOU/symlink window before the file is opened. Replace every use, choosing the temp API by lifetime:
Reported by internal static analysis and contributed upstream for the benefit of the wider community.
Assisted-by: GitHub Copilot CLI (Claude Opus 4.8)
I do not have commit access and will need someone to submit on my behalf.