gh-97612: Fix shell injection in get-remote-certificate.py (#97613)

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>
This commit is contained in:
Victor Stinner 2022-09-29 01:17:27 +02:00 committed by GitHub
parent a5f092f3c4
commit 83a0f44ffd
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 10 additions and 18 deletions

View File

@ -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.

View File

@ -15,8 +15,8 @@ import tempfile
def fetch_server_certificate (host, port): def fetch_server_certificate (host, port):
def subproc(cmd): def subproc(cmd):
from subprocess import Popen, PIPE, STDOUT from subprocess import Popen, PIPE, STDOUT, DEVNULL
proc = Popen(cmd, stdout=PIPE, stderr=STDOUT, shell=True) proc = Popen(cmd, stdout=PIPE, stderr=STDOUT, stdin=DEVNULL)
status = proc.wait() status = proc.wait()
output = proc.stdout.read() output = proc.stdout.read()
return status, output return status, output
@ -33,8 +33,8 @@ def fetch_server_certificate (host, port):
fp.write(m.group(1) + b"\n") fp.write(m.group(1) + b"\n")
try: try:
tn2 = (outfile or tempfile.mktemp()) tn2 = (outfile or tempfile.mktemp())
status, output = subproc(r'openssl x509 -in "%s" -out "%s"' % cmd = ['openssl', 'x509', '-in', tn, '-out', tn2]
(tn, tn2)) status, output = subproc(cmd)
if status != 0: if status != 0:
raise RuntimeError('OpenSSL x509 failed with status %s and ' raise RuntimeError('OpenSSL x509 failed with status %s and '
'output: %r' % (status, output)) 'output: %r' % (status, output))
@ -45,20 +45,9 @@ def fetch_server_certificate (host, port):
finally: finally:
os.unlink(tn) os.unlink(tn)
if sys.platform.startswith("win"): cmd = ['openssl', 's_client', '-connect', '%s:%s' % (host, port), '-showcerts']
tfile = tempfile.mktemp() status, output = subproc(cmd)
with open(tfile, "w") as fp:
fp.write("quit\n")
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))
if status != 0: if status != 0:
raise RuntimeError('OpenSSL connect failed with status %s and ' raise RuntimeError('OpenSSL connect failed with status %s and '
'output: %r' % (status, output)) 'output: %r' % (status, output))