[issue35755] Remove current directory from posixpath.defpath to enhance security

STINNER Victor added the comment: New changeset 394b991e41a2a4ce3afc8e6fde44de46e73bbb34 by Victor Stinner in branch '3.7': [3.7] bpo-35755: shutil.which() uses os.confstr("CS_PATH") (GH-12862) https://github.com/python/cpython/commit/394b991e41a2a4ce3afc8e6fde44de46e73bbb34 --

[issue35755] Remove current directory from posixpath.defpath to enhance security

Change by STINNER Victor : -- pull_requests: +12786 ___ Python tracker ___ ___ Python-bugs-list mailing list Unsubscribe:

[issue35755] Remove current directory from posixpath.defpath to enhance security

STINNER Victor added the comment: New changeset 197f0447e3bcfa4f529fedab09966d7e3d283979 by Victor Stinner in branch 'master': bpo-35755: Don't say "to mimick Unix which command behavior" (GH-12861) https://github.com/python/cpython/commit/197f0447e3bcfa4f529fedab09966d7e3d283979

[issue35755] Remove current directory from posixpath.defpath to enhance security

STINNER Victor added the comment: Anyway, I wrote PR 12861 to remove "to mimick Unix which command behavior". -- ___ Python tracker ___

[issue35755] Remove current directory from posixpath.defpath to enhance security

Change by STINNER Victor : -- pull_requests: +12785 ___ Python tracker ___ ___ Python-bugs-list mailing list Unsubscribe:

[issue35755] Remove current directory from posixpath.defpath to enhance security

STINNER Victor added the comment: > My point is that "which" implementations have different behavior I don't understand this point. Your example is consistent with what I saw on my Fedora 29 and the Python implementation that I just merged. Would you mind to elaborate which corner case is

[issue35755] Remove current directory from posixpath.defpath to enhance security

Jakub Wilk added the comment: (Note that in msg333835 another implementation, presumably GNU which, was tested.) My point is that "which" implementations have different behavior, so justifying anything with "which" compatibility is weird at best. You can't be compatible with all them.

[issue35755] Remove current directory from posixpath.defpath to enhance security

STINNER Victor added the comment: New changeset 2c4c02f8a876fcf084575dcaf857a0236c81261a by Victor Stinner in branch 'master': bpo-35755: Remove current directory from posixpath.defpath (GH-11586) https://github.com/python/cpython/commit/2c4c02f8a876fcf084575dcaf857a0236c81261a --

[issue35755] Remove current directory from posixpath.defpath to enhance security

STINNER Victor added the comment: CS_PATH value: * Fedora 29: "/usr/bin" * Ubuntu 16.04: "/bin:/usr/bin" It seems like the current directory is usually not part of the CS_PATH value. -- ___ Python tracker

[issue35755] Remove current directory from posixpath.defpath to enhance security

STINNER Victor added the comment: New changeset 228a3c99bdb2d02771bead66a0beabafad3a90d3 by Victor Stinner in branch 'master': bpo-35755: shutil.which() uses os.confstr("CS_PATH") (GH-12858) https://github.com/python/cpython/commit/228a3c99bdb2d02771bead66a0beabafad3a90d3 --

[issue35755] Remove current directory from posixpath.defpath to enhance security

STINNER Victor added the comment: find_executable() first looks if the program exists in the current directory. My PR doesn't change that. I have no opinion if it's a good thing or not, but I don't want to change that in this PR. If someone wants to change it, please open a separated issue

[issue35755] Remove current directory from posixpath.defpath to enhance security

STINNER Victor added the comment: My PR is consistent with the behavior you described in your zsh example, no? which doesn't find python if PATH is empty or equal to ":". -- ___ Python tracker

[issue35755] Remove current directory from posixpath.defpath to enhance security

Jakub Wilk added the comment: which(1) is not standardized, and there are many[*] implementations with different behavior in corner cases. For example, this happens with zsh 5.7.1 on Debian: % which python /usr/bin/python % PATH= which python python % PATH=. which python

[issue35755] Remove current directory from posixpath.defpath to enhance security

STINNER Victor added the comment: I wrote PR 12858 to os.confstr("CS_PATH") if available in shutil.which() and distutils.spawn.find_executable(), but also change the behavior when the PATH environment variable is set to an empty string: use an empty string, don't use os.confstr("CS_PATH")

[issue35755] Remove current directory from posixpath.defpath to enhance security

Change by STINNER Victor : -- pull_requests: +12782 ___ Python tracker ___ ___ Python-bugs-list mailing list Unsubscribe:

[issue35755] Remove current directory from posixpath.defpath to enhance security

Change by Jakub Wilk : -- nosy: +jwilk ___ Python tracker ___ ___ Python-bugs-list mailing list Unsubscribe:

[issue35755] Remove current directory from posixpath.defpath to enhance security

Alexey Izbyshev added the comment: Thanks for the info on CS_PATH, Victor. IMHO it'd make sense to use the libc-provided default PATH at least in shutil.which() since its intent is to emulate "which" from the default shell. -- ___ Python tracker

[issue35755] Remove current directory from posixpath.defpath to enhance security

STINNER Victor added the comment: Alexey Izbyshev: > Would it make sense to use os.confstr('CS_PATH') instead of a hardcoded path, > or is identical behavior on all POSIX platforms preferred to that? I didn't know this variable. man confstr says: _CS_PATH: A value for the PATH variable

[issue35755] Remove current directory from posixpath.defpath to enhance security

Serhiy Storchaka added the comment: This is how the which command behaves: $ /usr/bin/which python /usr/bin/python $ PATH= /usr/bin/which python $ PATH=. /usr/bin/which python ./python $ PATH=: /usr/bin/which python ./python I think shutil.which() should behave similarly unless there are

[issue35755] Remove current directory from posixpath.defpath to enhance security

STINNER Victor added the comment: I wrote PR 11586 to remove the current directory from os.defpath. I would prefer to first decide how the os, subprocess, shutil and distutils modules have to handle a PATH variable set to an empty string, before merging my PR. I would prefer to have the

[issue35755] Remove current directory from posixpath.defpath to enhance security

Change by STINNER Victor : -- keywords: +patch, patch, patch pull_requests: +11275, 11276, 11277 stage: -> patch review ___ Python tracker ___

[issue35755] Remove current directory from posixpath.defpath to enhance security

Change by STINNER Victor : -- keywords: +patch pull_requests: +11275 stage: -> patch review ___ Python tracker ___ ___

[issue35755] Remove current directory from posixpath.defpath to enhance security

Change by STINNER Victor : -- keywords: +patch, patch pull_requests: +11275, 11276 stage: -> patch review ___ Python tracker ___

[issue35755] Remove current directory from posixpath.defpath to enhance security

STINNER Victor added the comment: I'm working on PR but I found an issue. shutil.which() behaves differently than subprocess, distutils.spawn.find_executable() and os.execv() when PATH is set but set to an empty string: * os.get_exec_path() returns [''] * shutil.which() returns None: DON'T

[issue35755] Remove current directory from posixpath.defpath to enhance security

Christian Heimes added the comment: +1, /usr/bin:/bin sounds good to me. My /usr/include/paths.h has #define _PATH_DEFPATH "/usr/bin:/bin" and #define _PATH_STDPATH "/usr/bin:/bin:/usr/sbin:/sbin". The file is pretty old and has copyright from 89 and 93,

[issue35755] Remove current directory from posixpath.defpath to enhance security

Gregory P. Smith added the comment: I'm not arguing against this change, just trying to figure out where it came from in the first place. We should fix the value on all OSes. It would be a behavior change so probably only good for 3.8+. -- ___

[issue35755] Remove current directory from posixpath.defpath to enhance security

Gregory P. Smith added the comment: ntpath and macpath appear to have the same potential issue. They've basically had this defpath value forever. keep following it and you find a commit from 1994 https://github.com/python/cpython/commit/2979b01ff88ac4c5b316d9bf98edbaaaffac8e24 Changing them

[issue35755] Remove current directory from posixpath.defpath to enhance security

Alexey Izbyshev added the comment: Would it make sense to use os.confstr('CS_PATH') instead of a hardcoded path, or is identical behavior on all POSIX platforms preferred to that? -- nosy: +izbyshev ___ Python tracker

[issue35755] Remove current directory from posixpath.defpath to enhance security

Change by STINNER Victor : Added file: https://bugs.python.org/file48064/subprocess_curdir.py ___ Python tracker ___ ___ Python-bugs-list

[issue35755] Remove current directory from posixpath.defpath to enhance security

STINNER Victor added the comment: I wrote attached execv_curdir.py to check if os.execv() tries to find the executable in the current directory if it doesn't contain a directory: yes, it does. $ python3 execv_curdir.py execv() searchs in the current directory I also wrote attached

[issue35755] Remove current directory from posixpath.defpath to enhance security

New submission from STINNER Victor : Currently, posixpath.defpath is equal to: defpath = ':/bin:/usr/bin' It gives 3 directories: >>> posixpath.defpath.split(posixpath.pathsep) ['', '/bin', '/usr/bin'] where the empty string means "the current directory". Trying to locate an executable