bpo-29778: Ensure python3.dll is loaded from correct locations when Python is embedde...

Also enables using debug build of `python3_d.dll`
Reference: CVE-2020-15523
(cherry picked from commit dcbaa1b49cd9062fb9ba2b9d49555ac6cd8c60b5)

Co-authored-by: Steve Dower <steve.dower@python.org>

diff --git a/Lib/test/test_embed.py b/Lib/test/test_embed.py
index ec2b416da368d52329c80ca51e4b8a6823b05a64..64a26b9ffff071e4aee203f738c29cc43cc6c417 100644 (file)
--- a/Lib/test/test_embed.py
+++ b/Lib/test/test_embed.py
@@ -32,7 +32,7 @@ API_ISOLATED = 3
 def debug_build(program):
     program = os.path.basename(program)
     name = os.path.splitext(program)[0]
-    return name.endswith("_d")
+    return name.casefold().endswith("_d".casefold())
 
 
 def remove_python_envvars():
@@ -567,7 +567,7 @@ class InitConfigTests(EmbeddingTestsMixin, unittest.TestCase):
             if expected['stdio_errors'] is self.GET_DEFAULT_CONFIG:
                 expected['stdio_errors'] = 'surrogateescape'
 
-        if sys.platform == 'win32':
+        if MS_WINDOWS:
             default_executable = self.test_exe
         elif expected['program_name'] is not self.GET_DEFAULT_CONFIG:
             default_executable = os.path.abspath(expected['program_name'])
@@ -601,7 +601,7 @@ class InitConfigTests(EmbeddingTestsMixin, unittest.TestCase):
         pre_config = dict(configs['pre_config'])
         for key, value in list(expected.items()):
             if value is self.IGNORE_CONFIG:
-                del pre_config[key]
+                pre_config.pop(key, None)
                 del expected[key]
         self.assertEqual(pre_config, expected)
 
@@ -609,7 +609,7 @@ class InitConfigTests(EmbeddingTestsMixin, unittest.TestCase):
         config = dict(configs['config'])
         for key, value in list(expected.items()):
             if value is self.IGNORE_CONFIG:
-                del config[key]
+                config.pop(key, None)
                 del expected[key]
         self.assertEqual(config, expected)
 
@@ -684,6 +684,7 @@ class InitConfigTests(EmbeddingTestsMixin, unittest.TestCase):
         self.check_pre_config(configs, expected_preconfig)
         self.check_config(configs, expected_config)
         self.check_global_config(configs)
+        return configs
 
     def test_init_default_config(self):
         self.check_all_configs("test_init_initialize_config", api=API_COMPAT)
@@ -1035,6 +1036,7 @@ class InitConfigTests(EmbeddingTestsMixin, unittest.TestCase):
         }
         self.default_program_name(config)
         env = {'TESTPATH': os.path.pathsep.join(paths)}
+
         self.check_all_configs("test_init_setpath", config,
                                api=API_COMPAT, env=env,
                                ignore_stderr=True)
@@ -1092,12 +1094,18 @@ class InitConfigTests(EmbeddingTestsMixin, unittest.TestCase):
                 # Copy pythonXY.dll (or pythonXY_d.dll)
                 ver = sys.version_info
                 dll = f'python{ver.major}{ver.minor}'
+                dll3 = f'python{ver.major}'
                 if debug_build(sys.executable):
                     dll += '_d'
+                    dll3 += '_d'
                 dll += '.dll'
+                dll3 += '.dll'
                 dll = os.path.join(os.path.dirname(self.test_exe), dll)
+                dll3 = os.path.join(os.path.dirname(self.test_exe), dll3)
                 dll_copy = os.path.join(tmpdir, os.path.basename(dll))
+                dll3_copy = os.path.join(tmpdir, os.path.basename(dll3))
                 shutil.copyfile(dll, dll_copy)
+                shutil.copyfile(dll3, dll3_copy)
 
             # Copy Python program
             exec_copy = os.path.join(tmpdir, os.path.basename(self.test_exe))
@@ -1225,9 +1233,18 @@ class InitConfigTests(EmbeddingTestsMixin, unittest.TestCase):
                 config['base_prefix'] = pyvenv_home
                 config['prefix'] = pyvenv_home
             env = self.copy_paths_by_env(config)
-            self.check_all_configs("test_init_compat_config", config,
-                                   api=API_COMPAT, env=env,
-                                   ignore_stderr=True, cwd=tmpdir)
+            actual = self.check_all_configs("test_init_compat_config", config,
+                                            api=API_COMPAT, env=env,
+                                            ignore_stderr=True, cwd=tmpdir)
+            if MS_WINDOWS:
+                self.assertEqual(
+                    actual['windows']['python3_dll'],
+                    os.path.join(
+                        tmpdir,
+                        os.path.basename(self.EXPECTED_CONFIG['windows']['python3_dll'])
+                    )
+                )
+
 
     def test_global_pathconfig(self):
         # Test C API functions getting the path configuration:

diff --git a/Misc/NEWS.d/next/Security/2020-07-03-17-21-37.bpo-29778.cR_fGS.rst b/Misc/NEWS.d/next/Security/2020-07-03-17-21-37.bpo-29778.cR_fGS.rst
new file mode 100644 (file)
index 0000000..998ffb1
--- /dev/null
+++ b/Misc/NEWS.d/next/Security/2020-07-03-17-21-37.bpo-29778.cR_fGS.rst
@@ -0,0 +1,2 @@
+Ensure :file:`python3.dll` is loaded from correct locations when Python is
+embedded (CVE-2020-15523).

diff --git a/Modules/_testinternalcapi.c b/Modules/_testinternalcapi.c
index 3ea77e6934db1b09ed1fe15bb358dc333d3ad2ac..3a931cae1fc2eec6b3e58f5e197750511ad6fb65 100644 (file)
--- a/Modules/_testinternalcapi.c
+++ b/Modules/_testinternalcapi.c
@@ -12,10 +12,53 @@
 #include "pycore_initconfig.h"
 
 
+#ifdef MS_WINDOWS
+#include <windows.h>
+
+static int
+_add_windows_config(PyObject *configs)
+{
+    HMODULE hPython3;
+    wchar_t py3path[MAX_PATH];
+    PyObject *dict = PyDict_New();
+    PyObject *obj = NULL;
+    if (!dict) {
+        return -1;
+    }
+
+    hPython3 = GetModuleHandleW(PY3_DLLNAME);
+    if (hPython3 && GetModuleFileNameW(hPython3, py3path, MAX_PATH)) {
+        obj = PyUnicode_FromWideChar(py3path, -1);
+    } else {
+        obj = Py_None;
+        Py_INCREF(obj);
+    }
+    if (obj &&
+        !PyDict_SetItemString(dict, "python3_dll", obj) &&
+        !PyDict_SetItemString(configs, "windows", dict)) {
+        Py_DECREF(obj);
+        Py_DECREF(dict);
+        return 0;
+    }
+    Py_DECREF(obj);
+    Py_DECREF(dict);
+    return -1;
+}
+#endif
+
+
 static PyObject *
 get_configs(PyObject *self, PyObject *Py_UNUSED(args))
 {
-    return _Py_GetConfigsAsDict();
+    PyObject *dict = _Py_GetConfigsAsDict();
+#ifdef MS_WINDOWS
+    if (dict) {
+        if (_add_windows_config(dict) < 0) {
+            Py_CLEAR(dict);
+        }
+    }
+#endif
+    return dict;
 }
 
 

diff --git a/PC/getpathp.c b/PC/getpathp.c
index 53653c81fb45fad4ad28a566cb8a3141e0ed43cf..cf20b9f613c87b7f49eb3d6afbd297d3821247b9 100644 (file)
--- a/PC/getpathp.c
+++ b/PC/getpathp.c
@@ -130,8 +130,6 @@ typedef struct {
     wchar_t *machine_path;   /* from HKEY_LOCAL_MACHINE */
     wchar_t *user_path;      /* from HKEY_CURRENT_USER */
 
-    wchar_t *dll_path;
-
     const wchar_t *pythonpath_env;
 } PyCalculatePath;
 
@@ -167,27 +165,37 @@ reduce(wchar_t *dir)
 static int
 change_ext(wchar_t *dest, const wchar_t *src, const wchar_t *ext)
 {
-    size_t src_len = wcsnlen_s(src, MAXPATHLEN+1);
-    size_t i = src_len;
-    if (i >= MAXPATHLEN+1) {
-        Py_FatalError("buffer overflow in getpathp.c's reduce()");
-    }
+    if (src && src != dest) {
+        size_t src_len = wcsnlen_s(src, MAXPATHLEN+1);
+        size_t i = src_len;
+        if (i >= MAXPATHLEN+1) {
+            Py_FatalError("buffer overflow in getpathp.c's reduce()");
+        }
 
-    while (i > 0 && src[i] != '.' && !is_sep(src[i]))
-        --i;
+        while (i > 0 && src[i] != '.' && !is_sep(src[i]))
+            --i;
 
-    if (i == 0) {
-        dest[0] = '\0';
-        return -1;
-    }
+        if (i == 0) {
+            dest[0] = '\0';
+            return -1;
+        }
+
+        if (is_sep(src[i])) {
+            i = src_len;
+        }
 
-    if (is_sep(src[i])) {
-        i = src_len;
+        if (wcsncpy_s(dest, MAXPATHLEN+1, src, i)) {
+            dest[0] = '\0';
+            return -1;
+        }
+    } else {
+        wchar_t *s = wcsrchr(dest, L'.');
+        if (s) {
+            s[0] = '\0';
+        }
     }
 
-    if (wcsncpy_s(dest, MAXPATHLEN+1, src, i) ||
-        wcscat_s(dest, MAXPATHLEN+1, ext))
-    {
+    if (wcscat_s(dest, MAXPATHLEN+1, ext)) {
         dest[0] = '\0';
         return -1;
     }
@@ -344,6 +352,19 @@ search_for_prefix(wchar_t *prefix, const wchar_t *argv0_path, const wchar_t *lan
 }
 
 
+static int
+get_dllpath(wchar_t *dllpath)
+{
+#ifdef Py_ENABLE_SHARED
+    extern HANDLE PyWin_DLLhModule;
+    if (PyWin_DLLhModule && GetModuleFileNameW(PyWin_DLLhModule, dllpath, MAXPATHLEN)) {
+        return 0;
+    }
+#endif
+    return -1;
+}
+
+
 #ifdef Py_ENABLE_SHARED
 
 /* a string loaded from the DLL at startup.*/
@@ -516,27 +537,6 @@ done:
 #endif /* Py_ENABLE_SHARED */
 
 
-wchar_t*
-_Py_GetDLLPath(void)
-{
-    wchar_t dll_path[MAXPATHLEN+1];
-    memset(dll_path, 0, sizeof(dll_path));
-
-#ifdef Py_ENABLE_SHARED
-    extern HANDLE PyWin_DLLhModule;
-    if (PyWin_DLLhModule) {
-        if (!GetModuleFileNameW(PyWin_DLLhModule, dll_path, MAXPATHLEN)) {
-            dll_path[0] = 0;
-        }
-    }
-#else
-    dll_path[0] = 0;
-#endif
-
-    return _PyMem_RawWcsdup(dll_path);
-}
-
-
 static PyStatus
 get_program_full_path(_PyPathConfig *pathconfig)
 {
@@ -717,19 +717,17 @@ static int
 get_pth_filename(PyCalculatePath *calculate, wchar_t *filename,
                  const _PyPathConfig *pathconfig)
 {
-    if (calculate->dll_path[0]) {
-        if (!change_ext(filename, calculate->dll_path, L"._pth") &&
-            exists(filename))
-        {
-            return 1;
-        }
+    if (get_dllpath(filename) &&
+        !change_ext(filename, filename, L"._pth") &&
+        exists(filename))
+    {
+        return 1;
     }
-    if (pathconfig->program_full_path[0]) {
-        if (!change_ext(filename, pathconfig->program_full_path, L"._pth") &&
-            exists(filename))
-        {
-            return 1;
-        }
+    if (pathconfig->program_full_path[0] &&
+        !change_ext(filename, pathconfig->program_full_path, L"._pth") &&
+        exists(filename))
+    {
+        return 1;
     }
     return 0;
 }
@@ -1029,9 +1027,12 @@ calculate_path(PyCalculatePath *calculate, _PyPathConfig *pathconfig)
     wchar_t zip_path[MAXPATHLEN+1];
     memset(zip_path, 0, sizeof(zip_path));
 
-    change_ext(zip_path,
-               calculate->dll_path[0] ? calculate->dll_path : pathconfig->program_full_path,
-               L".zip");
+    if (get_dllpath(zip_path) || change_ext(zip_path, zip_path, L".zip"))
+    {
+        if (change_ext(zip_path, pathconfig->program_full_path, L".zip")) {
+            zip_path[0] = L'\0';
+        }
+    }
 
     calculate_home_prefix(calculate, argv0_path, zip_path, prefix);
 
@@ -1068,11 +1069,6 @@ calculate_init(PyCalculatePath *calculate, _PyPathConfig *pathconfig,
     calculate->home = pathconfig->home;
     calculate->path_env = _wgetenv(L"PATH");
 
-    calculate->dll_path = _Py_GetDLLPath();
-    if (calculate->dll_path == NULL) {
-        return _PyStatus_NO_MEMORY();
-    }
-
     calculate->pythonpath_env = config->pythonpath_env;
 
     return _PyStatus_OK();
@@ -1084,7 +1080,6 @@ calculate_free(PyCalculatePath *calculate)
 {
     PyMem_RawFree(calculate->machine_path);
     PyMem_RawFree(calculate->user_path);
-    PyMem_RawFree(calculate->dll_path);
 }
 
 
@@ -1094,7 +1089,6 @@ calculate_free(PyCalculatePath *calculate)
 
    - PyConfig.pythonpath_env: PYTHONPATH environment variable
    - _PyPathConfig.home: Py_SetPythonHome() or PYTHONHOME environment variable
-   - DLL path: _Py_GetDLLPath()
    - PATH environment variable
    - __PYVENV_LAUNCHER__ environment variable
    - GetModuleFileNameW(NULL): fully qualified path of the executable file of
@@ -1148,33 +1142,35 @@ int
 _Py_CheckPython3(void)
 {
     wchar_t py3path[MAXPATHLEN+1];
-    wchar_t *s;
     if (python3_checked) {
         return hPython3 != NULL;
     }
     python3_checked = 1;
 
     /* If there is a python3.dll next to the python3y.dll,
-       assume this is a build tree; use that DLL */
-    if (_Py_dll_path != NULL) {
-        wcscpy(py3path, _Py_dll_path);
-    }
-    else {
-        wcscpy(py3path, L"");
-    }
-    s = wcsrchr(py3path, L'\\');
-    if (!s) {
-        s = py3path;
+       use that DLL */
+    if (!get_dllpath(py3path)) {
+        reduce(py3path);
+        join(py3path, PY3_DLLNAME);
+        hPython3 = LoadLibraryExW(py3path, NULL, LOAD_LIBRARY_SEARCH_DEFAULT_DIRS);
+        if (hPython3 != NULL) {
+            return 1;
+        }
     }
-    wcscpy(s, L"\\python3.dll");
-    hPython3 = LoadLibraryExW(py3path, NULL, LOAD_WITH_ALTERED_SEARCH_PATH);
+
+    /* If we can locate python3.dll in our application dir,
+       use that DLL */
+    hPython3 = LoadLibraryExW(PY3_DLLNAME, NULL, LOAD_LIBRARY_SEARCH_APPLICATION_DIR);
     if (hPython3 != NULL) {
         return 1;
     }
 
-    /* Check sys.prefix\DLLs\python3.dll */
+    /* For back-compat, also search {sys.prefix}\DLLs, though
+       that has not been a normal install layout for a while */
     wcscpy(py3path, Py_GetPrefix());
-    wcscat(py3path, L"\\DLLs\\python3.dll");
-    hPython3 = LoadLibraryExW(py3path, NULL, LOAD_WITH_ALTERED_SEARCH_PATH);
+    if (py3path[0]) {
+        join(py3path, L"DLLs\\" PY3_DLLNAME);
+        hPython3 = LoadLibraryExW(py3path, NULL, LOAD_LIBRARY_SEARCH_DEFAULT_DIRS);
+    }
     return hPython3 != NULL;
 }

diff --git a/PCbuild/pyproject.props b/PCbuild/pyproject.props
index 59f2df4e0867b963f278cb9f60f62aee4f28bf19..360b4eda230dd96e466a32069b98f874b1a58e90 100644 (file)
--- a/PCbuild/pyproject.props
+++ b/PCbuild/pyproject.props
@@ -26,11 +26,12 @@
     <_PlatformPreprocessorDefinition>_WIN32;</_PlatformPreprocessorDefinition>
     <_PlatformPreprocessorDefinition Condition="$(Platform) == 'x64'">_WIN64;_M_X64;</_PlatformPreprocessorDefinition>
     <_PydPreprocessorDefinition Condition="$(TargetExt) == '.pyd'">Py_BUILD_CORE_MODULE;</_PydPreprocessorDefinition>
+    <_Py3NamePreprocessorDefinition>PY3_DLLNAME=L"$(Py3DllName)";</_Py3NamePreprocessorDefinition>
   </PropertyGroup>
   <ItemDefinitionGroup>
     <ClCompile>
       <AdditionalIncludeDirectories>$(PySourcePath)Include;$(PySourcePath)Include\internal;$(PySourcePath)PC;$(IntDir);%(AdditionalIncludeDirectories)</AdditionalIncludeDirectories>
-      <PreprocessorDefinitions>WIN32;$(_PlatformPreprocessorDefinition)$(_DebugPreprocessorDefinition)$(_PydPreprocessorDefinition)%(PreprocessorDefinitions)</PreprocessorDefinitions>
+      <PreprocessorDefinitions>WIN32;$(_Py3NamePreprocessorDefinition);$(_PlatformPreprocessorDefinition)$(_DebugPreprocessorDefinition)$(_PydPreprocessorDefinition)%(PreprocessorDefinitions)</PreprocessorDefinitions>
 
       <Optimization>MaxSpeed</Optimization>
       <IntrinsicFunctions>true</IntrinsicFunctions>

diff --git a/PCbuild/python.props b/PCbuild/python.props
index 6388d1b6426756402de9947369a381a265f0085b..bf6f3716ba51bce47c95f8b8cc08a14d6564a95c 100644 (file)
--- a/PCbuild/python.props
+++ b/PCbuild/python.props
@@ -203,6 +203,8 @@
     
     <!-- The name of the resulting pythonXY.dll (without the extension) -->
     <PyDllName>python$(MajorVersionNumber)$(MinorVersionNumber)$(PyDebugExt)</PyDllName>
+    <!-- The name of the resulting pythonX.dll (without the extension) -->
+    <Py3DllName>python3$(PyDebugExt)</Py3DllName>
 
     <!-- The version and platform tag to include in .pyd filenames -->
     <PydTag Condition="$(ArchName) == 'win32'">.cp$(MajorVersionNumber)$(MinorVersionNumber)-win32</PydTag>

diff --git a/Python/dynload_win.c b/Python/dynload_win.c
index 6deba1134e2a4bebd33b49ad989d210c722a7396..4896c6dc8c25dd0a6c826d0e3853d366a2aed541 100644 (file)
--- a/Python/dynload_win.c
+++ b/Python/dynload_win.c
@@ -174,9 +174,7 @@ dl_funcptr _PyImport_FindSharedFuncptrWindows(const char *prefix,
     char funcname[258], *import_python;
     const wchar_t *wpathname;
 
-#ifndef _DEBUG
     _Py_CheckPython3();
-#endif
 
     wpathname = _PyUnicode_AsUnicode(pathname);
     if (wpathname == NULL)

diff --git a/Python/pathconfig.c b/Python/pathconfig.c
index bf180976b55ab95f667a3588346607567e899714..60c104492d6468da7721c0487bc6b48e49235174 100644 (file)
--- a/Python/pathconfig.c
+++ b/Python/pathconfig.c
@@ -15,9 +15,6 @@ extern "C" {
 
 
 _PyPathConfig _Py_path_config = _PyPathConfig_INIT;
-#ifdef MS_WINDOWS
-wchar_t *_Py_dll_path = NULL;
-#endif
 
 
 static int
@@ -105,10 +102,6 @@ _PyPathConfig_ClearGlobal(void)
     _PyMem_SetDefaultAllocator(PYMEM_DOMAIN_RAW, &old_alloc);
 
     pathconfig_clear(&_Py_path_config);
-#ifdef MS_WINDOWS
-    PyMem_RawFree(_Py_dll_path);
-    _Py_dll_path = NULL;
-#endif
 
     PyMem_SetAllocator(PYMEM_DOMAIN_RAW, &old_alloc);
 }
@@ -145,31 +138,6 @@ _PyWideStringList_Join(const PyWideStringList *list, wchar_t sep)
 }
 
 
-#ifdef MS_WINDOWS
-/* Initialize _Py_dll_path on Windows. Do nothing on other platforms. */
-static PyStatus
-_PyPathConfig_InitDLLPath(void)
-{
-    if (_Py_dll_path != NULL) {
-        /* Already set: nothing to do */
-        return _PyStatus_OK();
-    }
-
-    PyMemAllocatorEx old_alloc;
-    _PyMem_SetDefaultAllocator(PYMEM_DOMAIN_RAW, &old_alloc);
-
-    _Py_dll_path = _Py_GetDLLPath();
-
-    PyMem_SetAllocator(PYMEM_DOMAIN_RAW, &old_alloc);
-
-    if (_Py_dll_path == NULL) {
-        return _PyStatus_NO_MEMORY();
-    }
-    return _PyStatus_OK();
-}
-#endif
-
-
 static PyStatus
 pathconfig_set_from_config(_PyPathConfig *pathconfig, const PyConfig *config)
 {
@@ -220,13 +188,6 @@ done:
 PyStatus
 _PyConfig_WritePathConfig(const PyConfig *config)
 {
-#ifdef MS_WINDOWS
-    PyStatus status = _PyPathConfig_InitDLLPath();
-    if (_PyStatus_EXCEPTION(status)) {
-        return status;
-    }
-#endif
-
     return pathconfig_set_from_config(&_Py_path_config, config);
 }
 
@@ -454,13 +415,6 @@ pathconfig_global_init(void)
 {
     PyStatus status;
 
-#ifdef MS_WINDOWS
-    status = _PyPathConfig_InitDLLPath();
-    if (_PyStatus_EXCEPTION(status)) {
-        Py_ExitStatusException(status);
-    }
-#endif
-
     if (_Py_path_config.module_search_path == NULL) {
         status = pathconfig_global_read(&_Py_path_config);
         if (_PyStatus_EXCEPTION(status)) {