classification
Title: [CVE-2020-15523] _Py_CheckPython3 uses uninitialized dllpath when embedder sets module path with Py_SetPath
Type: security Stage: resolved
Components: Windows Versions: Python 3.10, Python 3.9, Python 3.8, Python 3.7, Python 3.6, Python 3.5
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: steve.dower Nosy List: Tibor Csonka, anthonywee, eryksun, larry, lukasz.langa, miss-islington, ned.deily, paul.moore, steve.dower, tim.golden, vstinner, zach.ware
Priority: release blocker Keywords: patch

Created on 2017-03-10 04:58 by Tibor Csonka, last changed 2020-08-04 02:16 by larry. This issue is now closed.

Pull Requests
URL Status Linked Edit
PR 17818 merged anthonywee, 2020-01-04 02:13
PR 17871 merged miss-islington, 2020-01-06 16:59
PR 21297 merged steve.dower, 2020-07-03 17:48
PR 21298 merged steve.dower, 2020-07-03 19:54
PR 21306 merged vstinner, 2020-07-03 23:04
PR 21351 merged miss-islington, 2020-07-06 16:32
PR 21352 merged miss-islington, 2020-07-06 16:32
PR 21354 merged steve.dower, 2020-07-06 18:35
PR 21377 merged steve.dower, 2020-07-07 17:07
Messages (37)
msg289334 - (view) Author: Tibor Csonka (Tibor Csonka) Date: 2017-03-10 04:58
When Py_SetPath is used to set up module path at initialization, the Py_SetPath causes getpathp.c::calculate_path not to be called. However, calculate path is the only function calling getpathp.c::get_progpath which initializes the local dllpath static variable.

Later the interpreter tries to load python3.dll and uses dllpath which is empty by default. This empty path gets joined with \python3.dll and \DLLs\python3.dll which is used in the LoadLibraryExW resulting in loading python3.dll from the root location of the windows drive the application is running from.

The behavior was reproduced using PyInstaller but it is present in any embedding application which uses Py_SetPath.
msg289364 - (view) Author: Steve Dower (steve.dower) * (Python committer) Date: 2017-03-10 14:23
I thought we'd documented that if you set the path when embedding you should also set the program name, but perhaps not (didn't check just now). If not, we should do that.

We shouldn't be loading python3.dll anywhere. Are you sure that's in CPython? Do you have a reference to the source file?
msg289412 - (view) Author: Steve Dower (steve.dower) * (Python committer) Date: 2017-03-10 21:50
Ah, I see. We force load it in PC/getpathp.c to ensure that it's ours and not another version's python3.dll.

We should probably refactor the GetModuleFileNameW call into its own function so we can call it from anywhere we need.
msg343638 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2019-05-27 15:11
> When Py_SetPath is used to set up module path at initialization, the Py_SetPath causes getpathp.c::calculate_path not to be called. However, calculate path is the only function calling getpathp.c::get_progpath which initializes the local dllpath static variable.

I fixed this issue in Python 3.8 with this commit:

commit 410759fba80aded5247b693c60745aa16906f3bb
Author: Victor Stinner <vstinner@redhat.com>
Date:   Sat May 18 04:17:01 2019 +0200

    bpo-36763: Remove _PyCoreConfig.dll_path (GH-13402)

I modified Py_SetPath() like that:

-    new_config.dll_path = _PyMem_RawWcsdup(L"");
+    new_config.dll_path = _Py_GetDLLPath();

Py_SetPath() no longer sets dll_path to an empty string.

Since we only got one bug report and I believe that Tibor Csonka found a way to workaround the issue since he reported it, I close the issue.

Please reopen/comment the issue if you would like to get this issue fixed in Python 3.7 as well.

--

Moreover, the PEP 587 now has a better API to configure embedded Python. I just implemented this PEP in bpo-36763.
msg359136 - (view) Author: Anthony Wee (anthonywee) * Date: 2020-01-01 00:00
It looks like there has been a regression in the fix for this issue. 

The commit below introduced a NULL check which causes a call to _PyPathConfig_Init() to be skipped if _Py_dll_path == NULL. It seems like the check should be "if (_Py_dll_path != NULL)"?

https://github.com/python/cpython/commit/c422167749f92d4170203e996a2c619c818335ea#diff-87aed37b4704d4e1513be6378c9c7fe6R169
msg359245 - (view) Author: Steve Dower (steve.dower) * (Python committer) Date: 2020-01-03 18:33
> It looks like there has been a regression in the fix for this issue.

You're right. Care to create a pull request to fix it?
msg359438 - (view) Author: Steve Dower (steve.dower) * (Python committer) Date: 2020-01-06 16:57
New changeset 7b79dc9200a19ecbac667111dffd58e314be02a8 by Steve Dower (Anthony Wee) in branch 'master':
bpo-29778: Fix incorrect NULL check in _PyPathConfig_InitDLLPath() (GH-17818)
https://github.com/python/cpython/commit/7b79dc9200a19ecbac667111dffd58e314be02a8
msg359439 - (view) Author: Steve Dower (steve.dower) * (Python committer) Date: 2020-01-06 16:58
Thanks, Anthony! And congratulations on becoming a CPython contributor!
msg359440 - (view) Author: miss-islington (miss-islington) Date: 2020-01-06 17:17
New changeset a9a43c221bf3896ed1d1c2eee2531b7121cf78e4 by Miss Islington (bot) in branch '3.8':
bpo-29778: Fix incorrect NULL check in _PyPathConfig_InitDLLPath() (GH-17818)
https://github.com/python/cpython/commit/a9a43c221bf3896ed1d1c2eee2531b7121cf78e4
msg359441 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2020-01-06 17:22
Oops, I'm guilty of pushing this change! Sorry & thanks for the fix.

    if (_Py_dll_path == NULL) {
        /* Already set: nothing to do */
        return _PyStatus_OK();
    }
msg359498 - (view) Author: Anthony Wee (anthonywee) * Date: 2020-01-07 09:01
Thank you Steve!

I'm still seeing python3.dll being loaded from \DLLs\python3.dll.

_Py_CheckPython3() uses Py_GetPrefix() as a prefix for \DLLs\python3.dll.

It looks like Py_SetPath() sets the _Py_path_config.prefix to "", but I'm not seeing anything else set it to a real value?

https://github.com/python/cpython/blob/7b79dc9200a19ecbac667111dffd58e314be02a8/Python/pathconfig.c#L508
msg359544 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2020-01-07 20:56
> It looks like Py_SetPath() sets the _Py_path_config.prefix to "", but I'm not seeing anything else set it to a real value?

In the master branch, _Py_CheckPython3() doesn't use _Py_path_config.prefix.

_PyPathConfig_InitDLLPath() calls GetModuleFileNameW(PyWin_DLLhModule, dll_path, MAXPATHLEN) if PyWin_DLLhModule is initialized.

For example, _PyPathConfig_InitDLLPath() is called by Py_Initialize() and Py_SetPath().

PyWin_DLLhModule is initialized by DllMain().

The code in the 3.8 branch looks very similar (I backported my "Remove _PyPathConfig.dll_path" change to 3.8: commit 9f3dcf802eefeb5ab821ce3c7204ab46557d53d7).
msg359549 - (view) Author: Anthony Wee (anthonywee) * Date: 2020-01-07 21:22
Hm, I'm seeing _Py_CheckPython3() use Py_GetPrefix(), which uses _Py_path_config.prefix?

https://github.com/python/cpython/blob/c02b41b1fb115c87693530ea6a480b2e15460424/PC/getpathp.c#L1185
msg359550 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2020-01-07 21:29
> Hm, I'm seeing _Py_CheckPython3() use Py_GetPrefix(), which uses _Py_path_config.prefix?

Oh right, that's the initial issue:

> Later the interpreter tries to load python3.dll and uses dllpath which is empty by default. This empty path gets joined with \python3.dll and \DLLs\python3.dll which is used in the LoadLibraryExW resulting in loading python3.dll from the root location of the windows drive the application is running from.

I reopen the issue.
msg372555 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2020-06-29 09:05
_Py_CheckPython3() tries to load "python3.dll" from two directories:

* first one based on _Py_dll_path
* second one based on Py_GetPrefix()

I understand that LoadLibraryExW() must not be attempted if _Py_dll_path is empty, or if Py_GetPrefix() is empty. Am I right?
msg372556 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2020-06-29 09:09
I understand that Python 3.5, 3.6 and 3.7 are also affected. It's not a regression.

On Python 3.5, 3.6 and 3.7, when Py_SetPath(path) is called, Py_GetPrefix() also returns an empty path. So at least the directory based on Py_GetPrefix() should also be skipped on Python 3.5-3.7 if Py_GetPrefix() is empty, right?
msg372595 - (view) Author: Steve Dower (steve.dower) * (Python committer) Date: 2020-06-29 15:56
> I understand that LoadLibraryExW() must not be attempted if _Py_dll_path is empty, or if Py_GetPrefix() is empty. Am I right?

More likely those should never be empty. Perhaps sys.prefix is optional, but the DLL path is the current executing module, and should always be set.

I suspect you're right, that 3.7 is also affected. But earlier versions would only _not_ fill the DLL path for static (non-shared) builds.

It looks like Py_SetPath in 3.7 started clearing it unnecessarily, so that may be the cause.
msg372596 - (view) Author: Steve Dower (steve.dower) * (Python committer) Date: 2020-06-29 16:02
Sorry, I take that back. Earlier versions would indeed skip initialization in some cases.

I propose we deprecate the dll_path field in PathConfig and just get the path directly in the three places it's necessary. The path calculations have security exposure, so let's just avoid trying to manage additional state around it unnecessarily.

I'll work on a patch this week unless someone else gets to it first.
msg372961 - (view) Author: Steve Dower (steve.dower) * (Python committer) Date: 2020-07-03 21:24
Bumping to release blocker and adding RMs. Should definitely get this fix merged within the next week, and I don't want the next round of releases to go out without it.
msg373138 - (view) Author: Steve Dower (steve.dower) * (Python committer) Date: 2020-07-06 16:32
New changeset dcbaa1b49cd9062fb9ba2b9d49555ac6cd8c60b5 by Steve Dower in branch 'master':
bpo-29778: Ensure python3.dll is loaded from correct locations when Python is embedded (GH-21297)
https://github.com/python/cpython/commit/dcbaa1b49cd9062fb9ba2b9d49555ac6cd8c60b5
msg373142 - (view) Author: miss-islington (miss-islington) Date: 2020-07-06 16:52
New changeset 4981fe36c7477303de830e8dca929a02caaaffe4 by Miss Islington (bot) in branch '3.9':
bpo-29778: Ensure python3.dll is loaded from correct locations when Python is embedded (GH-21297)
https://github.com/python/cpython/commit/4981fe36c7477303de830e8dca929a02caaaffe4
msg373147 - (view) Author: Ɓukasz Langa (lukasz.langa) * (Python committer) Date: 2020-07-06 17:12
New changeset aa7f7756149a10c64d01f583b71e91814db886ab by Miss Islington (bot) in branch '3.8':
bpo-29778: Ensure python3.dll is loaded from correct locations when Python is embedded (GH-21297) (GH-21352)
https://github.com/python/cpython/commit/aa7f7756149a10c64d01f583b71e91814db886ab
msg373150 - (view) Author: Ned Deily (ned.deily) * (Python committer) Date: 2020-07-06 17:25
New changeset 110dd153662a13b8ae1bb06348e5b1f118ab26d7 by Steve Dower in branch '3.7':
[3.7] bpo-29778: Ensure python3.dll is loaded from correct locations when Python is embedded (GH-21297) (#21298)
https://github.com/python/cpython/commit/110dd153662a13b8ae1bb06348e5b1f118ab26d7
msg373155 - (view) Author: Ned Deily (ned.deily) * (Python committer) Date: 2020-07-06 18:55
New changeset 46cbf6148a46883110883488d3e9febbe46ba861 by Steve Dower in branch '3.6':
[3.6] bpo-29778: Ensure python3.dll is loaded from correct locations when Python is embedded (GH-21298) (#21354)
https://github.com/python/cpython/commit/46cbf6148a46883110883488d3e9febbe46ba861
msg373156 - (view) Author: Steve Dower (steve.dower) * (Python committer) Date: 2020-07-06 18:57
Fixes are in. Also adding the CVE number to the bug title.
msg373164 - (view) Author: Steve Dower (steve.dower) * (Python committer) Date: 2020-07-06 19:46
Announcement post: https://mail.python.org/archives/list/security-announce@python.org/thread/C5RIXC2ZIML3NOEIOGFPA6ISGU5L2QXL/

CVE-2020-15523 is an invalid search path in Python 3.6 and later on 
Windows. It occurs during Py_Initialize() when the runtime attempts to 
pre-load python3.dll. If Py_SetPath() has been called, the expected 
location is not set, and locations elsewhere on the user's system will 
be searched.

This issue is not triggered when running python.exe. It only applies 
when CPython has been embedded in another application.

Issue: https://bugs.python.org/issue29778
Patch: https://github.com/python/cpython/pull/21297

The next patched releases will be: 3.9.0b5, 3.8.4, 3.7.9 (source only), 
3.6.12 (source only)

Other than applying the patch, applications may mitigate the 
vulnerability by explicitly calling LoadLibrary() on their copy of 
python3.dll before calling Py_Initialize(). Even with the patch applied, 
applications should include a copy of python3.dll alongside their main 
Python DLL.

Thanks to Eric Gantumur for detecting and reporting the issue to the 
Python Security Response Team.
msg373213 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2020-07-07 09:31
FYI this vulnerability is now tracked by:
https://python-security.readthedocs.io/vuln/pysetpath-python-dll-path.html
msg373214 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2020-07-07 09:33
Steve: Python 3.5 is also vulnerable, no? This branch still gets security fixes, do you plan to backport the fix? I can do it if you are not available.
msg373228 - (view) Author: Steve Dower (steve.dower) * (Python committer) Date: 2020-07-07 16:08
> Python 3.5 is also vulnerable, no? This branch still gets security fixes, do you plan to backport the fix?

You're right. I thought because the backport tag was gone on GitHub that it was EOL already.

I can do the backport.
msg373257 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2020-07-07 22:20
New changeset 8f42748ded5e978fe8a924115179d45a74a6363b by Victor Stinner in branch 'master':
bpo-29778: test_embed tests the path configuration (GH-21306)
https://github.com/python/cpython/commit/8f42748ded5e978fe8a924115179d45a74a6363b
msg373618 - (view) Author: Steve Dower (steve.dower) * (Python committer) Date: 2020-07-13 18:20
Correction: the original discovery credit goes to Eran Shimony <Eran.Shimony@cyberark.com> and Ido Hoorvitch  <Ido.Hoorvitch@cyberark.com> from CyberArk.
msg373757 - (view) Author: Steve Dower (steve.dower) * (Python committer) Date: 2020-07-16 17:08
FYI, issue41304 fixed a regression in this patch in 3.7 and later. The regression shipped in 3.8.4 and 3.9.0b4, but will be fixed in the subsequent releases.
msg373789 - (view) Author: Larry Hastings (larry) * (Python committer) Date: 2020-07-17 01:54
I must have taken my stupid pills today.  Why is this considered a "security" "release blocker"?  If you can put files in the root of the hard drive where Windows was installed, surely you have other, easier attack vectors.
msg373919 - (view) Author: Eryk Sun (eryksun) * (Python triager) Date: 2020-07-18 21:01
> If you can put files in the root of the hard drive where Windows was
> installed, surely you have other, easier attack vectors.

A rooted path is resolved relative to the process working directory, and Python can be started with any current working directory. 

The default access control set on the root directory of a filesystem allows any authenticated user to create files or directories, such as "D:\python3.dll". That's if a filesystem even supports security. Removable drives are often formatted as FAT32 or exFAT, and FAT filesystems have no security.

The system drive (almost always "C:") has to be an NTFS filesystem, and its root directory is locked down a bit more. It's at high integrity level with a no-write-up rule for files, but not for directories. Only a logon at elevated integrity level (high or system level) can create "C:\python3.dll". OTOH, any authenticated user is still allowed to create a directory, such as "C:\DLLs", and is granted the right to create files in it such as "C:\DLLs\python3.dll".
msg374024 - (view) Author: Larry Hastings (larry) * (Python committer) Date: 2020-07-20 20:18
I still don't understand why this is considered a Python security problem.  If the user can put a malicious "python3.dll" at some arbitrary spot in the filesystem (e.g. a USB flash drive), and fool Python.exe into loading it, then surely they could put an arbitrary executable at that same spot and launch it directly.  And that seems way more straightforward.  Why would anyone bother with this?
msg374029 - (view) Author: Eryk Sun (eryksun) * (Python triager) Date: 2020-07-20 21:27
> I still don't understand why this is considered a Python security problem.
> If the user can put a malicious "python3.dll" at some arbitrary spot in 
> the filesystem (e.g. a USB flash drive), and fool Python.exe into loading
> it, then surely they could put an arbitrary executable at that same spot 
> and launch it directly.

What would be the point of adding an arbitrary executable in "C:\spam" or "D:\"? It's not in the system PATH, "App Paths", or any file-association template command. But if you can inject code into vulnerable processes that embed Python by simply creating "C:\DLLs\python3.dll", that seems like low-hanging fruit to me. Just wait for it to be run with administrator access, and then you can own the entire system.
msg374785 - (view) Author: Larry Hastings (larry) * (Python committer) Date: 2020-08-04 02:16
New changeset f205f1000a2d7f8b044caf281041b3705f293480 by Steve Dower in branch '3.5':
[3.5] bpo-29778: Ensure python3.dll is loaded from correct locations when Python is embedded (GH-21297) (#21377)
https://github.com/python/cpython/commit/f205f1000a2d7f8b044caf281041b3705f293480
History
Date User Action Args
2020-08-04 02:16:28larrysetmessages: + msg374785
2020-07-20 21:27:31eryksunsetmessages: + msg374029
2020-07-20 20:18:22larrysetmessages: + msg374024
2020-07-18 21:01:03eryksunsetnosy: + eryksun
messages: + msg373919
2020-07-17 01:54:45larrysetmessages: + msg373789
2020-07-16 17:08:49steve.dowersetmessages: + msg373757
2020-07-13 18:20:13steve.dowersetmessages: + msg373618
2020-07-07 22:20:44vstinnersetmessages: + msg373257
2020-07-07 17:07:31steve.dowersetpull_requests: + pull_request20520
2020-07-07 16:08:25steve.dowersetnosy: + larry

messages: + msg373228
versions: + Python 3.5
2020-07-07 09:33:05vstinnersetmessages: + msg373214
2020-07-07 09:31:32vstinnersetmessages: + msg373213
2020-07-06 19:46:06steve.dowersetmessages: + msg373164
2020-07-06 18:57:21steve.dowersetstatus: open -> closed
title: _Py_CheckPython3 uses uninitialized dllpath when embedder sets module path with Py_SetPath -> [CVE-2020-15523] _Py_CheckPython3 uses uninitialized dllpath when embedder sets module path with Py_SetPath
messages: + msg373156

resolution: fixed
stage: patch review -> resolved
2020-07-06 18:55:47ned.deilysetmessages: + msg373155
2020-07-06 18:35:29steve.dowersetpull_requests: + pull_request20500
2020-07-06 17:25:03ned.deilysetmessages: + msg373150
2020-07-06 17:12:23lukasz.langasetmessages: + msg373147
2020-07-06 16:52:17miss-islingtonsetmessages: + msg373142
2020-07-06 16:32:23miss-islingtonsetpull_requests: + pull_request20498
2020-07-06 16:32:15miss-islingtonsetpull_requests: + pull_request20497
2020-07-06 16:32:03steve.dowersetmessages: + msg373138
2020-07-03 23:04:47vstinnersetpull_requests: + pull_request20458
2020-07-03 22:34:38steve.dowersetpull_requests: - pull_request20454
2020-07-03 22:33:21steve.dowersetpull_requests: + pull_request20454
2020-07-03 21:24:37steve.dowersetpriority: normal -> release blocker
versions: - Python 3.5
nosy: + ned.deily, lukasz.langa

messages: + msg372961
2020-07-03 19:54:50steve.dowersetpull_requests: + pull_request20447
2020-07-03 17:48:30steve.dowersetkeywords: + patch
stage: needs patch -> patch review
pull_requests: + pull_request20446
2020-07-03 16:08:24steve.dowersetassignee: steve.dower
2020-06-29 16:02:57steve.dowersetmessages: + msg372596
2020-06-29 15:56:29steve.dowersetmessages: + msg372595
stage: resolved -> needs patch
2020-06-29 09:09:59vstinnersettype: behavior -> security
messages: + msg372556
versions: + Python 3.5, Python 3.6, Python 3.7, Python 3.10
2020-06-29 09:05:57vstinnersetmessages: + msg372555
2020-01-07 21:29:23vstinnersetstatus: closed -> open
resolution: fixed -> (no value)
messages: + msg359550
2020-01-07 21:22:58anthonyweesetmessages: + msg359549
2020-01-07 20:56:18vstinnersetmessages: + msg359544
2020-01-07 09:01:14anthonyweesetmessages: + msg359498
2020-01-06 17:22:26vstinnersetmessages: + msg359441
2020-01-06 17:17:43miss-islingtonsetnosy: + miss-islington
messages: + msg359440
2020-01-06 16:59:02miss-islingtonsetpull_requests: + pull_request17287
2020-01-06 16:58:32steve.dowersetversions: + Python 3.9
2020-01-06 16:58:20steve.dowersetmessages: + msg359439
2020-01-06 16:57:38steve.dowersetmessages: + msg359438
2020-01-04 02:13:52anthonyweesetpull_requests: + pull_request17244
2020-01-03 18:33:45steve.dowersetmessages: + msg359245
2020-01-01 00:00:46anthonyweesetnosy: + anthonywee
messages: + msg359136
2019-05-27 15:11:15vstinnersetstatus: open -> closed
stage: needs patch -> resolved
resolution: fixed
versions: + Python 3.8, - Python 3.5, Python 3.6, Python 3.7
2019-05-27 15:11:06vstinnersetnosy: + vstinner
messages: + msg343638
2017-03-10 21:50:35steve.dowersetstage: needs patch
type: behavior
versions: + Python 3.6, Python 3.7
2017-03-10 21:50:21steve.dowersetmessages: + msg289412
2017-03-10 14:23:36steve.dowersetmessages: + msg289364
2017-03-10 04:58:18Tibor Csonkacreate