From 5ed9ebc53b10efdebfdb633020e3e0e2af2d9a25 Mon Sep 17 00:00:00 2001 From: Steve Dower Date: Tue, 28 Jan 2020 20:54:33 +1100 Subject: [PATCH 1/2] bpo-39401: Avoid unsafe DLL load on Windows 7 and earlier --- .../next/Security/2020-01-28-20-54-09.bpo-39401.he7h_A.rst | 1 + PC/getpathp.c | 6 ++++-- 2 files changed, 5 insertions(+), 2 deletions(-) create mode 100644 Misc/NEWS.d/next/Security/2020-01-28-20-54-09.bpo-39401.he7h_A.rst diff --git a/Misc/NEWS.d/next/Security/2020-01-28-20-54-09.bpo-39401.he7h_A.rst b/Misc/NEWS.d/next/Security/2020-01-28-20-54-09.bpo-39401.he7h_A.rst new file mode 100644 index 00000000000000..78274acfcb7438 --- /dev/null +++ b/Misc/NEWS.d/next/Security/2020-01-28-20-54-09.bpo-39401.he7h_A.rst @@ -0,0 +1 @@ +Avoid unsafe DLL load at startup on Windows 7 and earlier. diff --git a/PC/getpathp.c b/PC/getpathp.c index 085caf195a9920..f85e3100ebc01e 100644 --- a/PC/getpathp.c +++ b/PC/getpathp.c @@ -252,7 +252,8 @@ static void join(wchar_t *buffer, const wchar_t *stuff) { if (_PathCchCombineEx_Initialized == 0) { - HMODULE pathapi = LoadLibraryW(L"api-ms-win-core-path-l1-1-0.dll"); + HMODULE pathapi = LoadLibraryExW(L"api-ms-win-core-path-l1-1-0.dll", NULL, + LOAD_LIBRARY_SEARCH_SYSTEM32); if (pathapi) { _PathCchCombineEx = (PPathCchCombineEx)GetProcAddress(pathapi, "PathCchCombineEx"); } @@ -288,7 +289,8 @@ canonicalize(wchar_t *buffer, const wchar_t *path) } if (_PathCchCanonicalizeEx_Initialized == 0) { - HMODULE pathapi = LoadLibraryW(L"api-ms-win-core-path-l1-1-0.dll"); + HMODULE pathapi = LoadLibraryExW(L"api-ms-win-core-path-l1-1-0.dll", NULL, + LOAD_LIBRARY_SEARCH_SYSTEM32); if (pathapi) { _PathCchCanonicalizeEx = (PPathCchCanonicalizeEx)GetProcAddress(pathapi, "PathCchCanonicalizeEx"); } From 4b0a3bceb4c6e086481372f98dc5aeb63e23a996 Mon Sep 17 00:00:00 2001 From: Steve Dower Date: Tue, 28 Jan 2020 21:11:05 +1100 Subject: [PATCH 2/2] Replace dynamic load with static import --- PC/getpathp.c | 57 ++++---------------------------------- PCbuild/pythoncore.vcxproj | 2 +- 2 files changed, 6 insertions(+), 53 deletions(-) diff --git a/PC/getpathp.c b/PC/getpathp.c index f85e3100ebc01e..3b65b35ce6146e 100644 --- a/PC/getpathp.c +++ b/PC/getpathp.c @@ -91,6 +91,7 @@ #endif #include +#include #include #ifdef HAVE_SYS_TYPES_H @@ -242,43 +243,14 @@ ismodule(wchar_t *filename, int update_filename) stuff as fits will be appended. */ -static int _PathCchCombineEx_Initialized = 0; -typedef HRESULT(__stdcall *PPathCchCombineEx) (PWSTR pszPathOut, size_t cchPathOut, - PCWSTR pszPathIn, PCWSTR pszMore, - unsigned long dwFlags); -static PPathCchCombineEx _PathCchCombineEx; - static void join(wchar_t *buffer, const wchar_t *stuff) { - if (_PathCchCombineEx_Initialized == 0) { - HMODULE pathapi = LoadLibraryExW(L"api-ms-win-core-path-l1-1-0.dll", NULL, - LOAD_LIBRARY_SEARCH_SYSTEM32); - if (pathapi) { - _PathCchCombineEx = (PPathCchCombineEx)GetProcAddress(pathapi, "PathCchCombineEx"); - } - else { - _PathCchCombineEx = NULL; - } - _PathCchCombineEx_Initialized = 1; - } - - if (_PathCchCombineEx) { - if (FAILED(_PathCchCombineEx(buffer, MAXPATHLEN+1, buffer, stuff, 0))) { - Py_FatalError("buffer overflow in getpathp.c's join()"); - } - } else { - if (!PathCombineW(buffer, buffer, stuff)) { - Py_FatalError("buffer overflow in getpathp.c's join()"); - } + if (FAILED(PathCchCombineEx(buffer, MAXPATHLEN+1, buffer, stuff, 0))) { + Py_FatalError("buffer overflow in getpathp.c's join()"); } } -static int _PathCchCanonicalizeEx_Initialized = 0; -typedef HRESULT(__stdcall *PPathCchCanonicalizeEx) (PWSTR pszPathOut, size_t cchPathOut, - PCWSTR pszPathIn, unsigned long dwFlags); -static PPathCchCanonicalizeEx _PathCchCanonicalizeEx; - /* Call PathCchCanonicalizeEx(path): remove navigation elements such as "." and ".." to produce a direct, well-formed path. */ static PyStatus @@ -288,27 +260,8 @@ canonicalize(wchar_t *buffer, const wchar_t *path) return _PyStatus_NO_MEMORY(); } - if (_PathCchCanonicalizeEx_Initialized == 0) { - HMODULE pathapi = LoadLibraryExW(L"api-ms-win-core-path-l1-1-0.dll", NULL, - LOAD_LIBRARY_SEARCH_SYSTEM32); - if (pathapi) { - _PathCchCanonicalizeEx = (PPathCchCanonicalizeEx)GetProcAddress(pathapi, "PathCchCanonicalizeEx"); - } - else { - _PathCchCanonicalizeEx = NULL; - } - _PathCchCanonicalizeEx_Initialized = 1; - } - - if (_PathCchCanonicalizeEx) { - if (FAILED(_PathCchCanonicalizeEx(buffer, MAXPATHLEN + 1, path, 0))) { - return INIT_ERR_BUFFER_OVERFLOW(); - } - } - else { - if (!PathCanonicalizeW(buffer, path)) { - return INIT_ERR_BUFFER_OVERFLOW(); - } + if (FAILED(PathCchCanonicalizeEx(buffer, MAXPATHLEN + 1, path, 0))) { + return INIT_ERR_BUFFER_OVERFLOW(); } return _PyStatus_OK(); } diff --git a/PCbuild/pythoncore.vcxproj b/PCbuild/pythoncore.vcxproj index f5be8aa4051e83..cfab2fa4e189c0 100644 --- a/PCbuild/pythoncore.vcxproj +++ b/PCbuild/pythoncore.vcxproj @@ -106,7 +106,7 @@ _Py_HAVE_ZLIB;%(PreprocessorDefinitions) - version.lib;shlwapi.lib;ws2_32.lib;%(AdditionalDependencies) + version.lib;shlwapi.lib;ws2_32.lib;pathcch.lib;%(AdditionalDependencies)