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

vstinner · calebshortt · web-flow · commit c7ec7808191f · 2022-10-05T14:56:13.000-07:00
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 83a0f44)
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
@@ -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
@@ -15,8 +15,8 @@
 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 strip_to_x509_cert(certfile_contents, outfile=None):
             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 strip_to_x509_cert(certfile_contents, outfile=None):
             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))

Original file line number	Diff line number	Diff line change
`@@ -0,0 +1,3 @@`
	`1`	+Fix a shell code injection vulnerability in the ``get-remote-certificate.py``
	`2`	+example script. The script no longer uses a shell to run ``openssl`` commands.
	`3`	`+Issue reported and initial fix by Caleb Shortt. Patch by Victor Stinner.`