diff options
author | Victor Stinner <vstinner@python.org> | 2022-10-05 23:56:13 +0200 |
---|---|---|
committer | GitHub <noreply@github.com> | 2022-10-05 14:56:13 -0700 |
commit | c7ec7808191fc29f75d96496b682bd34be770a8f (patch) | |
tree | 0620c3816b5db7bb01a82f8945d018b8d95a7782 | |
parent | fd82f16c1e4d9a0661e9a2250004032797652e69 (diff) | |
download | cpython-git-c7ec7808191fc29f75d96496b682bd34be770a8f.tar.gz |
[3.7] gh-97612: Fix shell injection in get-remote-certificate.py (#97613) (#97634)
Fix a shell code injection vulnerability in the
get-remote-certificate.py example script. The script no longer uses a
shell to run "openssl" commands. Issue reported and initial fix by
Caleb Shortt.
Remove the Windows code path to send "quit" on stdin to the "openssl
s_client" command: use DEVNULL on all platforms instead.
Co-authored-by: Caleb Shortt <caleb@rgauge.com>
(cherry picked from commit 83a0f44ffd8b398673ae56c310cf5768d359c341)
-rw-r--r-- | Misc/NEWS.d/next/Security/2022-09-28-12-10-57.gh-issue-97612.y6NvOQ.rst | 3 | ||||
-rwxr-xr-x | Tools/scripts/get-remote-certificate.py | 26 |
2 files changed, 10 insertions, 19 deletions
diff --git a/Misc/NEWS.d/next/Security/2022-09-28-12-10-57.gh-issue-97612.y6NvOQ.rst b/Misc/NEWS.d/next/Security/2022-09-28-12-10-57.gh-issue-97612.y6NvOQ.rst new file mode 100644 index 0000000000..2f113492d4 --- /dev/null +++ b/Misc/NEWS.d/next/Security/2022-09-28-12-10-57.gh-issue-97612.y6NvOQ.rst @@ -0,0 +1,3 @@ +Fix a shell code injection vulnerability in the ``get-remote-certificate.py`` +example script. The script no longer uses a shell to run ``openssl`` commands. +Issue reported and initial fix by Caleb Shortt. Patch by Victor Stinner. diff --git a/Tools/scripts/get-remote-certificate.py b/Tools/scripts/get-remote-certificate.py index 5811f202ed..24cc61f32d 100755 --- a/Tools/scripts/get-remote-certificate.py +++ b/Tools/scripts/get-remote-certificate.py @@ -15,8 +15,8 @@ import tempfile def fetch_server_certificate (host, port): def subproc(cmd): - from subprocess import Popen, PIPE, STDOUT - proc = Popen(cmd, stdout=PIPE, stderr=STDOUT, shell=True) + from subprocess import Popen, PIPE, STDOUT, DEVNULL + proc = Popen(cmd, stdout=PIPE, stderr=STDOUT, stdin=DEVNULL) status = proc.wait() output = proc.stdout.read() return status, output @@ -34,8 +34,8 @@ def fetch_server_certificate (host, port): fp.close() try: tn2 = (outfile or tempfile.mktemp()) - status, output = subproc(r'openssl x509 -in "%s" -out "%s"' % - (tn, tn2)) + cmd = ['openssl', 'x509', '-in', tn, '-out', tn2] + status, output = subproc(cmd) if status != 0: raise RuntimeError('OpenSSL x509 failed with status %s and ' 'output: %r' % (status, output)) @@ -47,21 +47,9 @@ def fetch_server_certificate (host, port): finally: os.unlink(tn) - if sys.platform.startswith("win"): - tfile = tempfile.mktemp() - fp = open(tfile, "w") - fp.write("quit\n") - fp.close() - try: - status, output = subproc( - 'openssl s_client -connect "%s:%s" -showcerts < "%s"' % - (host, port, tfile)) - finally: - os.unlink(tfile) - else: - status, output = subproc( - 'openssl s_client -connect "%s:%s" -showcerts < /dev/null' % - (host, port)) + cmd = ['openssl', 's_client', '-connect', '%s:%s' % (host, port), '-showcerts'] + status, output = subproc(cmd) + if status != 0: raise RuntimeError('OpenSSL connect failed with status %s and ' 'output: %r' % (status, output)) |