Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

tempfile.TemporaryDirectory fails removing dir in some edge cases related to symlinks [CVE-2023-6597] #91133

Closed
afeblot mannequin opened this issue Mar 10, 2022 · 6 comments
Labels
3.8 (EOL) end of life 3.9 only security fixes 3.10 only security fixes 3.11 only security fixes 3.12 bugs and security fixes 3.13 bugs and security fixes stdlib Python modules in the Lib dir type-bug An unexpected behavior, bug, or error type-security A security issue

Comments

@afeblot
Copy link
Mannequin

afeblot mannequin commented Mar 10, 2022

BPO 46977
Nosy @afeblot

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 = None
closed_at = None
created_at = <Date 2022-03-10.17:22:08.506>
labels = ['library', '3.9', 'type-crash']
title = 'tempfile.TemporaryDirectory fails removing dir in some edge cases related to symlinks'
updated_at = <Date 2022-03-10.17:25:32.378>
user = 'https://github.com/afeblot'

bugs.python.org fields:

activity = <Date 2022-03-10.17:25:32.378>
actor = 'afeblot'
assignee = 'none'
closed = False
closed_date = None
closer = None
components = ['Library (Lib)']
creation = <Date 2022-03-10.17:22:08.506>
creator = 'afeblot'
dependencies = []
files = []
hgrepos = []
issue_num = 46977
keywords = []
message_count = 1.0
messages = ['414871']
nosy_count = 1.0
nosy_names = ['afeblot']
pr_nums = []
priority = 'normal'
resolution = None
stage = None
status = 'open'
superseder = None
type = 'crash'
url = 'https://bugs.python.org/issue46977'
versions = ['Python 3.9']

Linked PRs

@afeblot
Copy link
Mannequin Author

afeblot mannequin commented Mar 10, 2022

#!/usr/bin/env python3

import os
import tempfile

def createUnremovableDir(workdir):
    print(workdir)
    os.mkdir(f'{workdir}/mydir')
    os.symlink('/bin/bash', f'{workdir}/mydir/mylink') # Symlink to a root owned file
    os.chmod(f'{workdir}/mydir', 0o555)

with tempfile.TemporaryDirectory() as workdir:
    createUnremovableDir(workdir)

Fails because tempfile.TemporaryDirectory._rmtree tries to execute os.chmod(path, 0o700) on the symlink, which by default tries to change the root owned file symlink target instead of the symlink itself:

/tmp/tmp1_dy42ef
Traceback (most recent call last):
  File "/usr/lib/python3.9/shutil.py", line 682, in _rmtree_safe_fd
    os.unlink(entry.name, dir_fd=topfd)
PermissionError: [Errno 13] Permission denied: 'mylink'

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "~/tempfile_demo_bug.py", line 13, in <module>
    createUnremovableDir(workdir)
  File "/usr/lib/python3.9/tempfile.py", line 969, in __exit__
    self.cleanup()
  File "/usr/lib/python3.9/tempfile.py", line 973, in cleanup
    self._rmtree(self.name)
  File "/usr/lib/python3.9/tempfile.py", line 955, in _rmtree
    _rmtree(name, onerror=onerror)
  File "/usr/lib/python3.9/shutil.py", line 727, in rmtree
    _rmtree_safe_fd(fd, path, onerror)
  File "/usr/lib/python3.9/shutil.py", line 664, in _rmtree_safe_fd
    _rmtree_safe_fd(dirfd, fullname, onerror)
  File "/usr/lib/python3.9/shutil.py", line 684, in _rmtree_safe_fd
    onerror(os.unlink, fullname, sys.exc_info())
  File "/usr/lib/python3.9/tempfile.py", line 941, in onerror
    resetperms(path)
  File "/usr/lib/python3.9/tempfile.py", line 936, in resetperms
    _os.chmod(path, 0o700)
PermissionError: [Errno 1] Operation not permitted: '/tmp/tmp1_dy42ef/mydir/mylink'

and leaves:

(.venv python 3.9.9) $ find /tmp/tmp1_dy42ef -ls
   148228      4 drwx------   3 myuser myuser     4096 Mar 10 16:54 /tmp/tmp1_dy42ef
   148229      4 drwx------   2 myuser myuser     4096 Mar 10 16:54 /tmp/tmp1_dy42ef/mydir
   148230      0 lrwxrwxrwx   1 myuser myuser        9 Mar 10 16:54 /tmp/tmp1_dy42ef/mydir/mylink -> /bin/bash

This fixes it:

#!/usr/bin/env python3

import os
import tempfile

def createUnremovableDir(workdir):
    print(workdir)
    os.mkdir(f'{workdir}/mydir')
    os.symlink('/bin/bash', f'{workdir}/mydir/mylink') # Symlink to a root owned file
    os.chmod(f'{workdir}/mydir', 0o555)

def _rmtree(cls, name, ignore_errors=False):
    def onerror(func, path, exc_info):
        if issubclass(exc_info[0], PermissionError):
            def resetperms(path):
                try:
                    if os.chflags in os.supports_follow_symlinks:  # This is the patch
                        os.chflags(path, 0, follow_symlinks=False) # This is the patch
                    elif not os.path.islink(path):                 # This is the patch
                        os.chflags(path, 0)
                except AttributeError:
                    pass
                if os.chmod in os.supports_follow_symlinks:        # This is the patch
                    os.chmod(path, 0o700, follow_symlinks=False)   # This is the patch
                elif not os.path.islink(path):                     # This is the patch
                    os.chmod(path, 0o700)

            try:
                if path != name:
                    resetperms(os.path.dirname(path))
                resetperms(path)

                try:
                    os.unlink(path)
                # PermissionError is raised on FreeBSD for directories
                except (IsADirectoryError, PermissionError):
                    cls._rmtree(path, ignore_errors=ignore_errors)
            except FileNotFoundError:
                pass
        elif issubclass(exc_info[0], FileNotFoundError):
            pass
        else:
            if not ignore_errors:
                raise

    shutil.rmtree(name, onerror=onerror)

# Monkey patch the class method tempfile.TemporaryDirectory._rmtree
from types import MethodType
import shutil
tempfile.TemporaryDirectory._rmtree = MethodType(_rmtree, tempfile.TemporaryDirectory)

with tempfile.TemporaryDirectory() as workdir:
    createUnremovableDir(workdir)

@afeblot afeblot mannequin added 3.9 only security fixes stdlib Python modules in the Lib dir type-crash A hard crash of the interpreter, possibly with a core dump labels Mar 10, 2022
@ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
@iritkatriel iritkatriel added type-bug An unexpected behavior, bug, or error and removed type-crash A hard crash of the interpreter, possibly with a core dump labels May 18, 2022
kwi-dk added a commit to kwi-dk/cpython that referenced this issue Dec 1, 2022
@serhiy-storchaka
Copy link
Member

I think that it is a security issue which allows an unprivileged user to reset permissions of files if they can run Python program that has higher privileges. There are be special conditions for this, but perhaps you can do this by making the program to unpack a special tarfile that contains symlinks out of the tree in temporary directory.

@serhiy-storchaka serhiy-storchaka added type-security A security issue 3.11 only security fixes 3.10 only security fixes 3.8 (EOL) end of life 3.12 bugs and security fixes 3.13 bugs and security fixes labels Dec 7, 2023
miss-islington pushed a commit to miss-islington/cpython that referenced this issue Dec 7, 2023
…up (pythonGH-99930)

(cherry picked from commit 81c16cd)

Co-authored-by: Søren Løvborg <[email protected]>
Co-authored-by: Serhiy Storchaka <[email protected]>
serhiy-storchaka added a commit to serhiy-storchaka/cpython that referenced this issue Dec 7, 2023
…n cleanup (pythonGH-99930)

(cherry picked from commit 81c16cd)

Co-authored-by: Søren Løvborg <[email protected]>
Co-authored-by: Serhiy Storchaka <[email protected]>
serhiy-storchaka added a commit to serhiy-storchaka/cpython that referenced this issue Dec 7, 2023
…n cleanup (pythonGH-99930)

(cherry picked from commit 81c16cd)

Co-authored-by: Søren Løvborg <[email protected]>
Co-authored-by: Serhiy Storchaka <[email protected]>
serhiy-storchaka added a commit to serhiy-storchaka/cpython that referenced this issue Dec 7, 2023
… cleanup (pythonGH-99930)

(cherry picked from commit 81c16cd)

Co-authored-by: Søren Løvborg <[email protected]>
Co-authored-by: Serhiy Storchaka <[email protected]>
serhiy-storchaka added a commit to serhiy-storchaka/cpython that referenced this issue Dec 7, 2023
… cleanup (pythonGH-99930)

(cherry picked from commit 81c16cd)

Co-authored-by: Søren Løvborg <[email protected]>
Co-authored-by: Serhiy Storchaka <[email protected]>
serhiy-storchaka added a commit that referenced this issue Dec 7, 2023
…nup (GH-99930) (GH-112838)

(cherry picked from commit 81c16cd)

Co-authored-by: Søren Løvborg <[email protected]>
Co-authored-by: Serhiy Storchaka <[email protected]>
serhiy-storchaka added a commit that referenced this issue Dec 7, 2023
…nup (GH-99930) (GH-112839)

(cherry picked from commit 81c16cd)

Co-authored-by: Søren Løvborg <[email protected]>
@gpshead gpshead changed the title tempfile.TemporaryDirectory fails removing dir in some edge cases related to symlinks tempfile.TemporaryDirectory fails removing dir in some edge cases related to symlinks [CVE-2023-6597] Dec 8, 2023
ambv pushed a commit that referenced this issue Jan 17, 2024
…up (GH-99930) (GH-112843)

(cherry picked from commit 81c16cd)

Co-authored-by: Søren Løvborg <[email protected]>
ambv pushed a commit that referenced this issue Jan 17, 2024
…up (GH-99930) (GH-112842)

(cherry picked from commit 81c16cd)

Co-authored-by: Søren Løvborg <[email protected]>
ambv pushed a commit that referenced this issue Jan 17, 2024
…nup (GH-99930) (GH-112840)

(cherry picked from commit 81c16cd)

Co-authored-by: Søren Løvborg <[email protected]>
aisk pushed a commit to aisk/cpython that referenced this issue Feb 11, 2024
@gpshead
Copy link
Member

gpshead commented Feb 21, 2024

the fixes were merged, releases will be going out soon. thanks for the report and PRs everyone!

@gpshead gpshead closed this as completed Feb 21, 2024
@sparrowt
Copy link
Contributor

sparrowt commented Apr 3, 2024

The GHSA page for this CVE-2023-6597 GHSA-797f-63wg-8chv states that it affects python "versions 3.12.2, 3.11.8, ..." but AFAICS those versions both contain the fix (see gh-91133 mentioned in 3.12.2 release notes and 3.11.8 release notes).

See also this comment on another CVE-fixing-PR where the same thing has happened.

@sethmlarson
Copy link
Contributor

As noted here, I have fixed the CVE record metadata: #109858 (comment)

@sparrowt
Copy link
Contributor

Thank you. Sadly GHSA-797f-63wg-8chv is still wrong :(

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3.8 (EOL) end of life 3.9 only security fixes 3.10 only security fixes 3.11 only security fixes 3.12 bugs and security fixes 3.13 bugs and security fixes stdlib Python modules in the Lib dir type-bug An unexpected behavior, bug, or error type-security A security issue
Projects
Status: Done
Development

No branches or pull requests

5 participants