# HG changeset patch # User Victor Stinner # Date 1413445365 -7200 # Parent 0b15b5371c0712a78002035eb27249ed04cd7b57 Issue #22636: avoid using a shell in the ctypes.util module Replace os.popen() with subprocess.Popen. If the "gcc", "cc" or "objdump" command is not available, the code was supposed to raise an OSError exception. But there was a bug in the code. The shell code returns the exit code 10 if the required command is missing, and the code tries to check for the status 10. The problem is that os.popen() doesn't return the exit code directly, but a status which should be processed by os.WIFEXITED() and os.WEXITSTATUS(). In practice, the exception was never raised. The OSError exception was not documented and ctypes.util.find_library() is expected to return None if the library is not found. diff -r 0b15b5371c07 Lib/ctypes/test/test_find.py --- a/Lib/ctypes/test/test_find.py Wed May 25 18:17:25 2011 +0200 +++ b/Lib/ctypes/test/test_find.py Tue May 10 12:34:17 2016 +0000 @@ -1,6 +1,7 @@ import unittest -import os +import os, os.path import sys +from test import test_support from ctypes import * from ctypes.util import find_library from ctypes.test import is_resource_enabled @@ -65,6 +66,11 @@ if self.gle: self.gle.gleGetJoinStyle + def test_shell_injection(self): + result = find_library('; echo Hello shell > ' + test_support.TESTFN) + self.assertFalse(os.path.lexists(test_support.TESTFN)) + self.assertIsNone(result) + # On platforms where the default shared library suffix is '.so', # at least some libraries can be loaded as attributes of the cdll # object, since ctypes now tries loading the lib again diff -r 0b15b5371c07 Lib/ctypes/util.py --- a/Lib/ctypes/util.py Wed May 25 18:17:25 2011 +0200 +++ b/Lib/ctypes/util.py Tue May 10 12:34:17 2016 +0000 @@ -1,7 +1,6 @@ -###################################################################### -# This file should be kept compatible with Python 2.3, see PEP 291. # -###################################################################### -import sys, os +import os +import subprocess +import sys # find_library(name) returns the pathname of a library, or None. if os.name == "nt": @@ -90,24 +89,23 @@ def _findLib_gcc(name): expr = r'[^\(\)\s]*lib%s\.[^\(\)\s]*' % re.escape(name) - fdout, ccout = tempfile.mkstemp() - os.close(fdout) - cmd = 'if type gcc >/dev/null 2>&1; then CC=gcc; elif type cc >/dev/null 2>&1; then CC=cc;else exit 10; fi;' \ - 'LANG=C LC_ALL=C $CC -Wl,-t -o ' + ccout + ' 2>&1 -l' + name + cmd = 'if type gcc >/dev/null 2>&1; then CC=gcc; elif type cc >/dev/null 2>&1; then CC=cc;else exit; fi;' \ + 'LANG=C LC_ALL=C $CC -Wl,-t -o "$2" 2>&1 -l"$1"' + + temp = tempfile.NamedTemporaryFile() try: - f = os.popen(cmd) - try: - trace = f.read() - finally: - rv = f.close() + proc = subprocess.Popen((cmd, '_findLib_gcc', name, temp.name), + shell=True, + stdout=subprocess.PIPE) + [trace, _] = proc.communicate() finally: try: - os.unlink(ccout) + temp.close() except OSError, e: + # ENOENT is raised if the file was already removed, which is + # the normal behaviour of GCC if linking fails if e.errno != errno.ENOENT: raise - if rv == 10: - raise OSError, 'gcc or cc command not found' res = re.search(expr, trace) if not res: return None @@ -119,13 +117,13 @@ def _get_soname(f): if not f: return None - cmd = "/usr/ccs/bin/dump -Lpv 2>/dev/null " + f - f = os.popen(cmd) - try: - data = f.read() - finally: - f.close() - res = re.search(r'\[.*\]\sSONAME\s+([^\s]+)', data) + + with open(os.devnull, "wb") as null: + proc = subprocess.Popen(("/usr/ccs/bin/dump", "-Lpv", f), + stdout=subprocess.PIPE, + stderr=null) + [data, _] = proc.communicate() + res = re.search(br'\[.*\]\sSONAME\s+([^\s]+)', data) if not res: return None return res.group(1) @@ -134,16 +132,12 @@ # assuming GNU binutils / ELF if not f: return None - cmd = 'if ! type objdump >/dev/null 2>&1; then exit 10; fi;' \ - "objdump -p -j .dynamic 2>/dev/null " + f - f = os.popen(cmd) - try: - dump = f.read() - finally: - rv = f.close() - if rv == 10: - raise OSError, 'objdump command not found' - res = re.search(r'\sSONAME\s+([^\s]+)', dump) + cmd = 'if ! type objdump >/dev/null 2>&1; then exit; fi;' \ + 'objdump -p -j .dynamic 2>/dev/null "$1"' + proc = subprocess.Popen((cmd, '_get_soname', f), shell=True, + stdout=subprocess.PIPE) + [dump, _] = proc.communicate() + res = re.search(br'\sSONAME\s+([^\s]+)', dump) if not res: return None return res.group(1) @@ -154,23 +148,25 @@ def _num_version(libname): # "libxyz.so.MAJOR.MINOR" => [ MAJOR, MINOR ] - parts = libname.split(".") + parts = libname.split(b".") nums = [] try: while parts: nums.insert(0, int(parts.pop())) except ValueError: pass - return nums or [ sys.maxint ] + return nums or [sys.maxint] def find_library(name): ename = re.escape(name) expr = r':-l%s\.\S+ => \S*/(lib%s\.\S+)' % (ename, ename) - f = os.popen('/sbin/ldconfig -r 2>/dev/null') - try: - data = f.read() - finally: - f.close() + + with open(os.devnull, 'wb') as null: + proc = subprocess.Popen(('/sbin/ldconfig', '-r'), + stdout=subprocess.PIPE, + stderr=null) + [data, _] = proc.communicate() + res = re.findall(expr, data) if not res: return _get_soname(_findLib_gcc(name)) @@ -183,16 +179,28 @@ if not os.path.exists('/usr/bin/crle'): return None + env = dict(os.environ) + env['LC_ALL'] = 'C' + if is64: - cmd = 'env LC_ALL=C /usr/bin/crle -64 2>/dev/null' + args = ('/usr/bin/crle', '-64') else: - cmd = 'env LC_ALL=C /usr/bin/crle 2>/dev/null' + args = ('/usr/bin/crle',) paths = None - for line in os.popen(cmd).readlines(): - line = line.strip() - if line.startswith('Default Library Path (ELF):'): - paths = line.split()[4] + with open(os.devnull, 'wb') as null: + proc = subprocess.Popen(args, + stdout=subprocess.PIPE, + stderr=null, + env=env) + try: + for line in proc.stdout: + line = line.strip() + if line.startswith(b'Default Library Path (ELF):'): + paths = line.split()[4] + finally: + proc.stdout.close() + proc.wait() if not paths: return None @@ -226,11 +234,16 @@ # XXX assuming GLIBC's ldconfig (with option -p) expr = r'\s+(lib%s\.[^\s]+)\s+\(%s' % (re.escape(name), abi_type) - f = os.popen('/sbin/ldconfig -p 2>/dev/null') - try: - data = f.read() - finally: - f.close() + + env = dict(os.environ) + env['LC_ALL'] = 'C' + env['LANG'] = 'C' + with open(os.devnull, 'wb') as null: + p = subprocess.Popen(['/sbin/ldconfig', '-p'], + stderr=null, + stdout=subprocess.PIPE, + env=env) + [data, _] = p.communicate() res = re.search(expr, data) if not res: return None diff -r 0b15b5371c07 Misc/NEWS --- a/Misc/NEWS Wed May 25 18:17:25 2011 +0200 +++ b/Misc/NEWS Tue May 10 12:34:17 2016 +0000 @@ -80,6 +80,9 @@ - Issue #12045: Avoid duplicate execution of command in ctypes.util._get_soname(). Patch by Sijin Joseph. +- Issue #22636: Avoid shell injection problems with + ctypes.util.find_library(). + - Issue #26960: Backported #16270 from Python 3 to Python 2, to prevent urllib from hanging when retrieving certain FTP files.