[3.7] bpo-35755: shutil.which() uses os.confstr("CS_PATH") (GH-12862)

* bpo-35755: shutil.which() uses os.confstr("CS_PATH") (GH-12858)

shutil.which() and distutils.spawn.find_executable() now use
os.confstr("CS_PATH") if available instead of os.defpath, if the PATH
environment variable is not set.

Don't use os.confstr("CS_PATH") nor os.defpath if the PATH
environment variable is set to an empty string.

Changes:

* find_executable() now starts by checking for the executable in the
  current working directly case. Add an explicit
  "if not path: return None".
* Add tests for PATH='' (empty string), PATH=':' and for PATHEXT.

(cherry picked from commit 228a3c9)

* bpo-35755: Remove current directory from posixpath.defpath (GH-11586)

Document the change in a NEWS entry of the Security category.

(cherry picked from commit 2c4c02f)

Author: vstinner

Lib/distutils/spawn.py

@@ -172,21 +172,32 @@ def find_executable(executable, path=None):
     A string listing directories separated by 'os.pathsep'; defaults to
     os.environ['PATH'].  Returns the complete filename or None if not found.
     """
-    if path is None:
-        path = os.environ.get('PATH', os.defpath)
-
-    paths = path.split(os.pathsep)
-    base, ext = os.path.splitext(executable)
-
+    _, ext = os.path.splitext(executable)
     if (sys.platform == 'win32') and (ext != '.exe'):
         executable = executable + '.exe'
 
-    if not os.path.isfile(executable):
-        for p in paths:
-            f = os.path.join(p, executable)
-            if os.path.isfile(f):
-                # the file exists, we have a shot at spawn working
-                return f
-        return None
-    else:
+    if os.path.isfile(executable):
         return executable
+
+    if path is None:
+        path = os.environ.get('PATH', None)
+        if path is None:
+            try:
+                path = os.confstr("CS_PATH")
+            except (AttributeError, ValueError):
+                # os.confstr() or CS_PATH is not available
+                path = os.defpath
+        # bpo-35755: Don't use os.defpath if the PATH environment variable is
+        # set to an empty string
+
+    # PATH='' doesn't match, whereas PATH=':' looks in the current directory
+    if not path:
+        return None
+
+    paths = path.split(os.pathsep)
+    for p in paths:
+        f = os.path.join(p, executable)
+        if os.path.isfile(f):
+            # the file exists, we have a shot at spawn working
+            return f
+    return None

Lib/distutils/tests/test_spawn.py

@@ -87,11 +87,52 @@ def test_find_executable(self):
             rv = find_executable(dont_exist_program , path=tmp_dir)
             self.assertIsNone(rv)
 
-            # test os.defpath: missing PATH environment variable
+            # PATH='': no match, except in the current directory
             with test_support.EnvironmentVarGuard() as env:
-                with mock.patch('distutils.spawn.os.defpath', tmp_dir):
-                    env.pop('PATH')
+                env['PATH'] = ''
+                with unittest.mock.patch('distutils.spawn.os.confstr',
+                                         return_value=tmp_dir, create=True), \
+                     unittest.mock.patch('distutils.spawn.os.defpath',
+                                         tmp_dir):
+                    rv = find_executable(program)
+                    self.assertIsNone(rv)
+
+                    # look in current directory
+                    with test_support.change_cwd(tmp_dir):
+                        rv = find_executable(program)
+                        self.assertEqual(rv, program)
+
+            # PATH=':': explicitly looks in the current directory
+            with test_support.EnvironmentVarGuard() as env:
+                env['PATH'] = os.pathsep
+                with unittest.mock.patch('distutils.spawn.os.confstr',
+                                         return_value='', create=True), \
+                     unittest.mock.patch('distutils.spawn.os.defpath', ''):
+                    rv = find_executable(program)
+                    self.assertIsNone(rv)
+
+                    # look in current directory
+                    with test_support.change_cwd(tmp_dir):
+                        rv = find_executable(program)
+                        self.assertEqual(rv, program)
+
+            # missing PATH: test os.confstr("CS_PATH") and os.defpath
+            with test_support.EnvironmentVarGuard() as env:
+                env.pop('PATH', None)
+
+                # without confstr
+                with unittest.mock.patch('distutils.spawn.os.confstr',
+                                         side_effect=ValueError,
+                                         create=True), \
+                     unittest.mock.patch('distutils.spawn.os.defpath',
+                                         tmp_dir):
+                    rv = find_executable(program)
+                    self.assertEqual(rv, filename)
 
+                # with confstr
+                with unittest.mock.patch('distutils.spawn.os.confstr',
+                                         return_value=tmp_dir, create=True), \
+                     unittest.mock.patch('distutils.spawn.os.defpath', ''):
                     rv = find_executable(program)
                     self.assertEqual(rv, filename)
 

Lib/posixpath.py

@@ -18,7 +18,7 @@
 extsep = '.'
 sep = '/'
 pathsep = ':'
-defpath = ':/bin:/usr/bin'
+defpath = '/bin:/usr/bin'
 altsep = None
 devnull = '/dev/null'
 

Lib/shutil.py

@@ -1138,7 +1138,17 @@ def _access_check(fn, mode):
         return None
 
     if path is None:
-        path = os.environ.get("PATH", os.defpath)
+        path = os.environ.get("PATH", None)
+        if path is None:
+            try:
+                path = os.confstr("CS_PATH")
+            except (AttributeError, ValueError):
+                # os.confstr() or CS_PATH is not available
+                path = os.defpath
+        # bpo-35755: Don't use os.defpath if the PATH environment variable is
+        # set to an empty string
+
+    # PATH='' doesn't match, whereas PATH=':' looks in the current directory
     if not path:
         return None
     path = path.split(os.pathsep)

Lib/test/test_shutil.py

@@ -1500,6 +1500,57 @@ def test_environ_path(self):
             rv = shutil.which(self.file)
             self.assertEqual(rv, self.temp_file.name)
 
+    def test_environ_path_empty(self):
+        # PATH='': no match
+        with support.EnvironmentVarGuard() as env:
+            env['PATH'] = ''
+            with unittest.mock.patch('os.confstr', return_value=self.dir, \
+                                     create=True), \
+                 support.swap_attr(os, 'defpath', self.dir), \
+                 support.change_cwd(self.dir):
+                rv = shutil.which(self.file)
+                self.assertIsNone(rv)
+
+    def test_environ_path_cwd(self):
+        expected_cwd = os.path.basename(self.temp_file.name)
+        if sys.platform == "win32":
+            curdir = os.curdir
+            if isinstance(expected_cwd, bytes):
+                curdir = os.fsencode(curdir)
+            expected_cwd = os.path.join(curdir, expected_cwd)
+
+        # PATH=':': explicitly looks in the current directory
+        with support.EnvironmentVarGuard() as env:
+            env['PATH'] = os.pathsep
+            with unittest.mock.patch('os.confstr', return_value=self.dir, \
+                                     create=True), \
+                 support.swap_attr(os, 'defpath', self.dir):
+                rv = shutil.which(self.file)
+                self.assertIsNone(rv)
+
+                # look in current directory
+                with support.change_cwd(self.dir):
+                    rv = shutil.which(self.file)
+                    self.assertEqual(rv, expected_cwd)
+
+    def test_environ_path_missing(self):
+        with support.EnvironmentVarGuard() as env:
+            env.pop('PATH', None)
+
+            # without confstr
+            with unittest.mock.patch('os.confstr', side_effect=ValueError, \
+                                     create=True), \
+                 support.swap_attr(os, 'defpath', self.dir):
+                rv = shutil.which(self.file)
+            self.assertEqual(rv, self.temp_file.name)
+
+            # with confstr
+            with unittest.mock.patch('os.confstr', return_value=self.dir, \
+                                     create=True), \
+                 support.swap_attr(os, 'defpath', ''):
+                rv = shutil.which(self.file)
+            self.assertEqual(rv, self.temp_file.name)
+
     def test_empty_path(self):
         base_dir = os.path.dirname(self.dir)
         with support.change_cwd(path=self.dir), \
@@ -1514,6 +1565,23 @@ def test_empty_path_no_PATH(self):
             rv = shutil.which(self.file)
             self.assertIsNone(rv)
 
+    @unittest.skipUnless(sys.platform == "win32", 'test specific to Windows')
+    def test_pathext(self):
+        ext = ".xyz"
+        temp_filexyz = tempfile.NamedTemporaryFile(dir=self.temp_dir,
+                                                   prefix="Tmp2", suffix=ext)
+        os.chmod(temp_filexyz.name, stat.S_IXUSR)
+        self.addCleanup(temp_filexyz.close)
+
+        # strip path and extension
+        program = os.path.basename(temp_filexyz.name)
+        program = os.path.splitext(program)[0]
+
+        with support.EnvironmentVarGuard() as env:
+            env['PATHEXT'] = ext
+            rv = shutil.which(program, path=self.temp_dir)
+            self.assertEqual(rv, temp_filexyz.name)
+
 
 class TestMove(unittest.TestCase):
 

Misc/NEWS.d/next/Library/2019-04-16-17-50-39.bpo-35755.Fg4EXb.rst

@@ -0,0 +1,5 @@
+:func:`shutil.which` and :func:`distutils.spawn.find_executable` now use
+``os.confstr("CS_PATH")`` if available instead of :data:`os.defpath`, if the
+``PATH`` environment variable is not set. Moreover, don't use
+``os.confstr("CS_PATH")`` nor :data:`os.defpath` if the ``PATH`` environment
+variable is set to an empty string.

Misc/NEWS.d/next/Security/2019-01-17-10-03-48.bpo-35755.GmllIs.rst

@@ -0,0 +1,5 @@
+:func:`shutil.which` now uses ``os.confstr("CS_PATH")`` if available and if the
+:envvar:`PATH` environment variable is not set. Remove also the current
+directory from :data:`posixpath.defpath`. On Unix, :func:`shutil.which` and the
+:mod:`subprocess` module no longer search the executable in the current
+directory if the :envvar:`PATH` environment variable is not set.