homepage

This issue tracker has been migrated to GitHub , and is currently read-only.
For more information, see the GitHub FAQs in the Python's Developer Guide.

classification
Title: avoid using a shell in ctypes.util: replace os.popen with subprocess
Type: behavior Stage: resolved
Components: ctypes Versions: Python 3.6, Python 3.5, Python 2.7
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: Nosy List: Arfrever, barry, koobs, martin.panter, python-dev, serhiy.storchaka, vstinner
Priority: normal Keywords: patch

Created on 2014年10月14日 22:32 by vstinner, last changed 2022年04月11日 14:58 by admin. This issue is now closed.

Files
File name Uploaded Description Edit
ctypes_util_popen.patch vstinner, 2014年10月14日 22:32 review
ctypes_util_popen-2.patch vstinner, 2014年10月16日 10:16 review
ctypes_util_popen-3.py3.patch martin.panter, 2016年05月10日 12:01 review
ctypes_util_popen-3.py2.patch martin.panter, 2016年05月10日 12:42 review
dump martin.panter, 2016年05月10日 12:51 /usr/ccs/bin/dump
ldconfig martin.panter, 2016年05月10日 12:53 /sbin/ldconfig -r (BSD)
crle martin.panter, 2016年05月10日 12:54 /usr/bin/crle
ctypes_util_popen-4.py2.patch martin.panter, 2016年06月10日 04:16 review
ctypes_util_popen-5.py3.patch martin.panter, 2016年06月10日 06:20 review
ctypes_util_popen-5.py2.patch martin.panter, 2016年06月14日 03:30 review
ctypes_util_popen-6.py2.patch martin.panter, 2016年06月14日 09:56 review
Messages (25)
msg229363 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2014年10月14日 22:32
Attached patch modifies the ctypes.util module to not use a shell: it replaces os.open() with subprocess.Popen on Linux.
Running a shell is slower and is more vulnerable to code injection.
I only modified code path on Linux right now. They are still calls to os.popen() on sunos5, freebsd, openbsd and dragonfly.
msg229520 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2014年10月16日 10:16
Updated patch which address also BSD and Solaris systems.
I also changed the behaviour when a required command is missing: return None instead of raising an OSError.
In the current code, when a command is missing, the shell scripts return the exit code 10. The Python codes checks for the exit code 10, but in fact os.popen() returns a status, not directly the exit code. So the OSError was never raised.
I don't know if it's better to return None instead of raising an error? It changes the behaviour, can it break backward compatibility?
msg255522 - (view) Author: Martin Panter (martin.panter) * (Python committer) Date: 2015年11月28日 03:27
See Issue 25751 for some demo exploits on Linux, if anyone wants inspiration for test cases. Maybe this should be applied as a bug fix. I haven’t looked at the patch, other than confirming it removes all five os.popen() calls.
msg255552 - (view) Author: Martin Panter (martin.panter) * (Python committer) Date: 2015年11月28日 22:01
I think it is better to return None without an exception, to keep the current behaviour, and because that’s what the documentation implies.
msg255635 - (view) Author: Martin Panter (martin.panter) * (Python committer) Date: 2015年12月01日 01:56
Marking for bug fix in 2.7, requested in Issue 25751.
msg265111 - (view) Author: Martin Panter (martin.panter) * (Python committer) Date: 2016年05月08日 04:43
There are a few review comments that probably need addressing.
msg265242 - (view) Author: Martin Panter (martin.panter) * (Python committer) Date: 2016年05月10日 12:01
I merged Victor’s patch with the current code and addressed most of the comments:
* restore re.escape()
* single "-l" + name argument
* copy with dict(os.environ)
* redirect GCC stderr=STDOUT
* changed tempfile cleanup to try / finally
I also added a test case.
I kept Victor’s behaviour of not raising OSError when the command is missing. I think this should be considered separately, and only changed for 3.6+, if at all. The buggy code was added in Issue 4861.
I only have Linux and GCC, but I briefly tested each platform-specific branch by hacking the "if" statements and creating mock crle, ldconfig, etc commands, so I am somewhat confident that everything is still working.
msg265243 - (view) Author: Martin Panter (martin.panter) * (Python committer) Date: 2016年05月10日 12:42
Here is a possible patch for Python 2. One snag is that ctypes is currently supposed to be compatible with Python 2.3, but subprocess was added in 2.4. The patch assumes it is okay to lift that compatibility restriction.
The main differences are:
* shutil.which() does not exist in Python 2. In _findLib_gcc() and the Gnu version of _get_soname(), I restored the mini shell script that runs "type" to check if commands are available. However I pass the library and file names as proper arguments, rather than inserting them into the shell syntax.
* subprocess.DEVNULL does not exist. Manually opened os.devnull where necessary.
* There was an extra Popen conversion for the Gnu "ldconfig -p" call. In Python 3, this was already converted thanks to revision 19d9f0a177de (Issue 11258).
* No context manager support for Popen objects. Instead, use communicate() where appropriate, or manually close and wait.
Again, I tested the Python 2 patch on Linux, but with mock platform-specific commands to exercise each new Popen() call.
msg265244 - (view) Author: Martin Panter (martin.panter) * (Python committer) Date: 2016年05月10日 12:51
Uploading the fake commands I used for testing.
msg266051 - (view) Author: Martin Panter (martin.panter) * (Python committer) Date: 2016年05月22日 05:30
FTR the Python 2.3 compatibility restriction was lifted; see <https://mail.python.org/pipermail/python-dev/2016-May/144502.html>.
msg268081 - (view) Author: Martin Panter (martin.panter) * (Python committer) Date: 2016年06月10日 04:16
Updated Python 2 patch merged with recent changes.
I will commit at least the Python 3 version soon, because the existing code sets a bad example for potential additions (Issue 26439).
msg268085 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2016年06月10日 05:02
It looks to me that the command used in _findLib_gcc always fails.
$ LANG=C LC_ALL=C gcc -Wl,-t -o ttt -lc
/usr/bin/ld: mode elf_i386
/usr/lib/gcc/i686-linux-gnu/5/../../../i386-linux-gnu/crt1.o
/usr/lib/gcc/i686-linux-gnu/5/../../../i386-linux-gnu/crti.o
/usr/lib/gcc/i686-linux-gnu/5/crtbegin.o
/lib/i386-linux-gnu/libc.so.6
(/usr/lib/i386-linux-gnu/libc_nonshared.a)elf-init.oS
/lib/i386-linux-gnu/ld-linux.so.2
/lib/i386-linux-gnu/ld-linux.so.2
-lgcc_s (/usr/lib/gcc/i686-linux-gnu/5/libgcc_s.so)
/lib/i386-linux-gnu/libc.so.6
/lib/i386-linux-gnu/ld-linux.so.2
-lgcc_s (/usr/lib/gcc/i686-linux-gnu/5/libgcc_s.so)
/usr/lib/gcc/i686-linux-gnu/5/crtend.o
/usr/lib/gcc/i686-linux-gnu/5/../../../i386-linux-gnu/crtn.o
/usr/lib/gcc/i686-linux-gnu/5/../../../i386-linux-gnu/crt1.o: In function `_start':
(.text+0x18): undefined reference to `main'
/usr/bin/ld: link errors found, deleting executable `ttt'
collect2: error: ld returned 1 exit status
Is it OK?
msg268087 - (view) Author: Martin Panter (martin.panter) * (Python committer) Date: 2016年06月10日 05:22
Yes it is okay. The code is compiling a dummy file without main(), just to see what libraries GCC tries to link with it. It is only interested in extracting the line matching *libc.so.*, which in your case should be
/lib/i386-linux-gnu/libc.so.6
So you should find that ctypes.util._findLib_gcc("c") still returns this path, even though the compile command technically fails.
msg268092 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2016年06月10日 05:58
Maybe the failure should be explained in a comment? (Sorry I din't read the
patch.)
msg268094 - (view) Author: Martin Panter (martin.panter) * (Python committer) Date: 2016年06月10日 06:20
Yes a comment sounds like a good idea. Here is a new Py 3 patch.
msg268098 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2016年06月10日 06:58
ctypes_util_popen-5.py3.patch LGTM.
msg268505 - (view) Author: Roundup Robot (python-dev) (Python triager) Date: 2016年06月14日 03:03
New changeset 0715d403cae2 by Martin Panter in branch '3.5':
Issue #22636: avoid using a shell in the ctypes.util module
https://hg.python.org/cpython/rev/0715d403cae2
New changeset 60613ecad578 by Martin Panter in branch 'default':
Issue #22636: Merge ctypes.util shell injection fixes from 3.5
https://hg.python.org/cpython/rev/60613ecad578 
msg268508 - (view) Author: Martin Panter (martin.panter) * (Python committer) Date: 2016年06月14日 03:30
Updated Py 2 patch to v5 with the added GCC comment
msg268510 - (view) Author: Martin Panter (martin.panter) * (Python committer) Date: 2016年06月14日 03:53
An Open Indiana buildbot failed. The old code let the shell print any errors about missing programs to /dev/null, so I will change the subprocess calls to handle OSError.
======================================================================
ERROR: setUpModule (ctypes.test.test_loading)
----------------------------------------------------------------------
Traceback (most recent call last):
 File "/export/home/buildbot/32bits/3.5.cea-indiana-x86/build/Lib/ctypes/test/test_loading.py", line 19, in setUpModule
 libc_name = find_library("c")
 File "/export/home/buildbot/32bits/3.5.cea-indiana-x86/build/Lib/ctypes/util.py", line 238, in find_library
 return _get_soname(_findLib_crle(name, is64) or _findLib_gcc(name))
 File "/export/home/buildbot/32bits/3.5.cea-indiana-x86/build/Lib/ctypes/util.py", line 145, in _get_soname
 stderr=subprocess.DEVNULL)
 File "/export/home/buildbot/32bits/3.5.cea-indiana-x86/build/Lib/subprocess.py", line 947, in __init__
 restore_signals, start_new_session)
 File "/export/home/buildbot/32bits/3.5.cea-indiana-x86/build/Lib/subprocess.py", line 1551, in _execute_child
 raise child_exception_type(errno_num, err_msg)
FileNotFoundError: [Errno 2] No such file or directory: '/usr/ccs/bin/dump'
msg268516 - (view) Author: Roundup Robot (python-dev) (Python triager) Date: 2016年06月14日 04:40
New changeset 96d297e9a8a8 by Martin Panter in branch '3.5':
Issue #22636: Handle OSError from subprocess, e.g. if command not found
https://hg.python.org/cpython/rev/96d297e9a8a8
New changeset a6a36bb6ee50 by Martin Panter in branch 'default':
Issue #22636: Merge ctypes.util from 3.5
https://hg.python.org/cpython/rev/a6a36bb6ee50 
msg268548 - (view) Author: Martin Panter (martin.panter) * (Python committer) Date: 2016年06月14日 09:56
Updated Py 2 patch to handle OSError when shell=True is not used
msg268668 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2016年06月16日 13:25
ctypes_util_popen-6.py2.patch LGTM.
msg268754 - (view) Author: Roundup Robot (python-dev) (Python triager) Date: 2016年06月18日 02:02
New changeset a09ae70f3489 by Victor Stinner in branch '2.7':
Issue #22636: Avoid using a shell in the ctypes.util module
https://hg.python.org/cpython/rev/a09ae70f3489 
msg268755 - (view) Author: Martin Panter (martin.panter) * (Python committer) Date: 2016年06月18日 03:19
Sorry about impersonating your name as committer Victor. I have been fixing this problem in recent patches, but because I imported your patch a while ago I forgot about it.
msg268766 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2016年06月18日 06:05
No problemo.
History
Date User Action Args
2022年04月11日 14:58:09adminsetgithub: 66826
2016年06月18日 06:05:44vstinnersetmessages: + msg268766
2016年06月18日 03:19:15martin.pantersetstatus: open -> closed
resolution: fixed
messages: + msg268755

stage: patch review -> resolved
2016年06月18日 02:02:49python-devsetmessages: + msg268754
2016年06月16日 13:25:17serhiy.storchakasetmessages: + msg268668
2016年06月14日 09:56:14martin.pantersetfiles: + ctypes_util_popen-6.py2.patch

messages: + msg268548
2016年06月14日 04:40:33python-devsetmessages: + msg268516
2016年06月14日 03:53:44martin.pantersetmessages: + msg268510
2016年06月14日 03:30:59martin.pantersetfiles: + ctypes_util_popen-5.py2.patch

messages: + msg268508
2016年06月14日 03:03:43python-devsetnosy: + python-dev
messages: + msg268505
2016年06月10日 06:58:16serhiy.storchakasetmessages: + msg268098
2016年06月10日 06:20:24martin.pantersetfiles: + ctypes_util_popen-5.py3.patch

messages: + msg268094
2016年06月10日 05:58:23vstinnersetmessages: + msg268092
2016年06月10日 05:22:39martin.pantersetmessages: + msg268087
2016年06月10日 05:02:26serhiy.storchakasetmessages: + msg268085
2016年06月10日 04:33:52martin.panterlinkissue26439 dependencies
2016年06月10日 04:16:38martin.pantersetfiles: + ctypes_util_popen-4.py2.patch

messages: + msg268081
2016年05月22日 05:30:24martin.pantersetmessages: + msg266051
2016年05月10日 13:05:16martin.panterlinkissue6338 dependencies
2016年05月10日 12:54:10martin.pantersetfiles: + crle
2016年05月10日 12:53:21martin.pantersetfiles: + ldconfig
2016年05月10日 12:51:38martin.pantersetfiles: + dump

messages: + msg265244
2016年05月10日 12:42:43martin.pantersetfiles: + ctypes_util_popen-3.py2.patch

messages: + msg265243
2016年05月10日 12:01:49martin.pantersetfiles: + ctypes_util_popen-3.py3.patch

stage: needs patch -> patch review
messages: + msg265242
versions: - Python 3.4
2016年05月08日 04:43:28martin.pantersetmessages: + msg265111
stage: patch review -> needs patch
2015年12月01日 01:56:30martin.pantersettype: enhancement -> behavior
messages: + msg255635
versions: + Python 2.7, Python 3.4, Python 3.6
2015年11月28日 22:01:06martin.pantersetmessages: + msg255552
2015年11月28日 03:29:09martin.panterlinkissue25751 superseder
2015年11月28日 03:28:16martin.pantersetcomponents: + ctypes
2015年11月28日 03:27:25martin.pantersetnosy: + martin.panter

messages: + msg255522
stage: patch review
2014年11月06日 22:49:12barrysetnosy: + barry
2014年10月22日 10:16:02vstinnersetnosy: + serhiy.storchaka
2014年10月16日 11:41:55pitrousetnosy: + koobs
2014年10月16日 10:16:54vstinnersetfiles: + ctypes_util_popen-2.patch

messages: + msg229520
2014年10月14日 23:36:28Arfreversetnosy: + Arfrever
2014年10月14日 22:32:55vstinnercreate

AltStyle によって変換されたページ (->オリジナル) /