Skip to content

[CVE-2020-15523] _Py_CheckPython3 uses uninitialized dllpath when embedder sets module path with Py_SetPath #73964

Closed

Description

@TiborCsonka
BPO 29778
Nosy @pfmoore, @vstinner, @larryhastings, @tjguk, @ned-deily, @ambv, @zware, @eryksun, @zooba, @miss-islington, @anthonywee
PRs
  • bpo-29778: Fix incorrect NULL check #17818
  • [3.8] bpo-29778: Fix incorrect NULL check in _PyPathConfig_InitDLLPath() (GH-17818) #17871
  • bpo-29778: Ensure python3.dll is loaded from correct locations when Python is embedded #21297
  • [3.7] bpo-29778: Ensure python3.dll is loaded from correct locations when Python is embedded (GH-21297) #21298
  • bpo-29778: test_embed tests the path configuration #21306
  • [3.9] bpo-29778: Ensure python3.dll is loaded from correct locations when Python is embedded (GH-21297) #21351
  • [3.8] bpo-29778: Ensure python3.dll is loaded from correct locations when Python is embedded (GH-21297) #21352
  • [3.6] bpo-29778: Ensure python3.dll is loaded from correct locations when Python is embedded (GH-21298) #21354
  • [3.5] bpo-29778: Ensure python3.dll is loaded from correct locations when Python is embedded (GH-21297) #21377
  • Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.

    Show more details

    GitHub fields:

    assignee = 'https://github.com/zooba'
    closed_at = <Date 2020-07-06.18:57:21.099>
    created_at = <Date 2017-03-10.04:58:18.536>
    labels = ['type-security', '3.8', '3.9', '3.10', 'release-blocker', '3.7', 'OS-windows']
    title = '[CVE-2020-15523] _Py_CheckPython3 uses uninitialized dllpath when embedder sets module path with Py_SetPath'
    updated_at = <Date 2020-08-04.02:16:28.217>
    user = 'https://bugs.python.org/TiborCsonka'

    bugs.python.org fields:

    activity = <Date 2020-08-04.02:16:28.217>
    actor = 'larry'
    assignee = 'steve.dower'
    closed = True
    closed_date = <Date 2020-07-06.18:57:21.099>
    closer = 'steve.dower'
    components = ['Windows']
    creation = <Date 2017-03-10.04:58:18.536>
    creator = 'Tibor Csonka'
    dependencies = []
    files = []
    hgrepos = []
    issue_num = 29778
    keywords = ['patch']
    message_count = 37.0
    messages = ['289334', '289364', '289412', '343638', '359136', '359245', '359438', '359439', '359440', '359441', '359498', '359544', '359549', '359550', '372555', '372556', '372595', '372596', '372961', '373138', '373142', '373147', '373150', '373155', '373156', '373164', '373213', '373214', '373228', '373257', '373618', '373757', '373789', '373919', '374024', '374029', '374785']
    nosy_count = 12.0
    nosy_names = ['paul.moore', 'vstinner', 'larry', 'tim.golden', 'ned.deily', 'lukasz.langa', 'zach.ware', 'eryksun', 'steve.dower', 'Tibor Csonka', 'miss-islington', 'anthonywee']
    pr_nums = ['17818', '17871', '21297', '21298', '21306', '21351', '21352', '21354', '21377']
    priority = 'release blocker'
    resolution = 'fixed'
    stage = 'resolved'
    status = 'closed'
    superseder = None
    type = 'security'
    url = 'https://bugs.python.org/issue29778'
    versions = ['Python 3.5', 'Python 3.6', 'Python 3.7', 'Python 3.8', 'Python 3.9', 'Python 3.10']

    Activity

    1. TiborCsonka commented on Mar 10, 2017

      TiborCsonkamannequin
      MannequinAuthor

      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.

    2. zooba commented on Mar 10, 2017

      @zooba
      Member

      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?

    3. zooba commented on Mar 10, 2017

      @zooba
      Member

      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.

    4. vstinner commented on May 27, 2019

      @vstinner
      Member

      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 410759f
      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.

    5. added and removed on May 27, 2019
    6. anthonywee commented on Jan 1, 2020

      anthonyweemannequin
      Mannequin

      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)"?

      c422167#diff-87aed37b4704d4e1513be6378c9c7fe6R169

    7. zooba commented on Jan 3, 2020

      @zooba
      Member

      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?

    8. zooba commented on Jan 6, 2020

      @zooba
      Member

      New changeset 7b79dc9 by Steve Dower (Anthony Wee) in branch 'master':
      bpo-29778: Fix incorrect NULL check in _PyPathConfig_InitDLLPath() (GH-17818)
      7b79dc9

    9. zooba commented on Jan 6, 2020

      @zooba
      Member

      Thanks, Anthony! And congratulations on becoming a CPython contributor!

    10. 24 remaining items

    11. changed the 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[/+] on Jul 6, 2020
    12. zooba commented on Jul 6, 2020

      @zooba
      Member

      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: #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.

    13. vstinner commented on Jul 7, 2020

      @vstinner
      Member
    14. vstinner commented on Jul 7, 2020

      @vstinner
      Member

      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.

    15. zooba commented on Jul 7, 2020

      @zooba
      Member

      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.

    16. vstinner commented on Jul 7, 2020

      @vstinner
      Member

      New changeset 8f42748 by Victor Stinner in branch 'master':
      bpo-29778: test_embed tests the path configuration (GH-21306)
      8f42748

    17. zooba commented on Jul 13, 2020

      @zooba
      Member

      Correction: the original discovery credit goes to Eran Shimony <Eran.Shimony@cyberark.com> and Ido Hoorvitch <Ido.Hoorvitch@cyberark.com> from CyberArk.

    18. zooba commented on Jul 16, 2020

      @zooba
      Member

      FYI, bpo-41304 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.

    19. larryhastings commented on Jul 17, 2020

      @larryhastings
      Contributor

      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.

    20. eryksun commented on Jul 18, 2020

      @eryksun
      Contributor

      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".

    21. larryhastings commented on Jul 20, 2020

      @larryhastings
      Contributor

      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?

    22. eryksun commented on Jul 20, 2020

      @eryksun
      Contributor

      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.

    23. larryhastings commented on Aug 4, 2020

      @larryhastings
      Contributor

      New changeset f205f10 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) (bpo-21377)
      f205f10

    24. transferred this issue fromon Apr 10, 2022
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

    Metadata

    Metadata

    Assignees

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions