René's URL Explorer Experiment


Title: git.util.rmtree can change permissions outside tree on Unix (chmod traverses symlinks) · Issue #1738 · gitpython-developers/GitPython · GitHub

Open Graph Title: git.util.rmtree can change permissions outside tree on Unix (chmod traverses symlinks) · Issue #1738 · gitpython-developers/GitPython

X Title: git.util.rmtree can change permissions outside tree on Unix (chmod traverses symlinks) · Issue #1738 · gitpython-developers/GitPython

Description: Overview On Unix-like systems, git.util.rmtree will change permissions outside the tree being removed, under some circumstances. This occurs specifically with git.util.rmtree; it is not a flaw inherited from shutil.rmtree. It does not re...

Open Graph Description: Overview On Unix-like systems, git.util.rmtree will change permissions outside the tree being removed, under some circumstances. This occurs specifically with git.util.rmtree; it is not a flaw inhe...

X Description: Overview On Unix-like systems, git.util.rmtree will change permissions outside the tree being removed, under some circumstances. This occurs specifically with git.util.rmtree; it is not a flaw inhe...

Opengraph URL: https://github.com/gitpython-developers/GitPython/issues/1738

X: @github

direct link

Domain: github.com


Hey, it has json ld scripts:
{"@context":"https://schema.org","@type":"DiscussionForumPosting","headline":"git.util.rmtree can change permissions outside tree on Unix (chmod traverses symlinks)","articleBody":"### Overview\r\n\r\nOn Unix-like systems, [`git.util.rmtree`](https://github.com/gitpython-developers/GitPython/blob/74cc671826122f14951de48e7027c58eacaabe25/git/util.py#L200-L225) will change permissions *outside* the tree being removed, under some circumstances. This occurs specifically with `git.util.rmtree`; it is not a flaw inherited from [`shutil.rmtree`](https://docs.python.org/3/library/shutil.html#shutil.rmtree). It does not require a race condition, and the effect is easy to produce if one has control of the files in a directory that `git.util.rmtree` will later be used to remove, though this must include control over file permissions (or related metadata) somewhere within that tree. It could happen unintentionally as well as intentionally, but I believe unintentional occurrence is uncommon, at least in uses of `git.util.rmtree` from within GitPython.\r\n\r\nThis happens because, when `git.util.rmtree` cannot delete a file, it [calls](https://github.com/gitpython-developers/GitPython/blob/74cc671826122f14951de48e7027c58eacaabe25/git/util.py#L211) `os.chmod(path, stat.S_IWUSR)` on the file and tries again. **But `os.chmod` dereferences symlinks when called that way on a Unix-like system.** The effect is to change permissions on the target, which may be outside the tree. It adds write permissions, and removes some other permissions. Yet, while this seems like a case of [CWE-59](https://cwe.mitre.org/data/definitions/59.html), I'm *somewhat* reluctant to consider this a security vulnerability, because it only adds permissions for the file's owner, who is already capable of using `chmod` to add them. But there are some other concerns, detailed below in \"Is this a vulnerability?\" This bug should not be confused with [race conditions](https://docs.python.org/3/library/shutil.html#shutil.rmtree.avoids_symlink_attacks) affecting `rmtree` on some systems that can cause the wrong files to be deleted; this bug is about the wrong files having their permissions changed.\r\n\r\n**I believe this is straightforward to fix, with no breaking changes.** Although useful on Windows, on Unix-like systems that `os.chmod` call is unnecessary and, in cases where permissions do need to be changed to allow a file to be deleted, ineffective. So it can be run conditionally, on Windows only. Its ineffectiveness on Unix-like systems is thus a blessing in disguise, as otherwise making it run only on Windows could be a breaking change. I have proposed this fix, and regression tests, in #1739.\r\n\r\n### The cause\r\n\r\nOn all operating systems, `git.util.rmtree` currently calls `shutil.rmtree` with a callback that attempts to change a file's permissions to be writable and retry deletion:\r\n\r\nhttps://github.com/gitpython-developers/GitPython/blob/74cc671826122f14951de48e7027c58eacaabe25/git/util.py#L208-L214\r\n\r\nThe intent of the `os.chmod` call is to make the file at `path` writable. The problem is that, on Unix-like systems, calling `os.chmod` without passing `follow_symlinks=False` only changes the file at `path` when it is not a symbolic link. When the file at `path` is a symbolic link, it is dereferenced, and its (direct or indirect) target has its permissions changed instead.\r\n\r\n`os.chmod` behaves this way on Unix-like systems because, on such systems, unless `follow_symlinks=False` is passed, `os.chmod` uses [chmod(2)](https://man7.org/linux/man-pages/man2/chmod.2.html), which follows symlinks:\r\n\r\n\u003e `chmod()` changes the mode of the file specified whose pathname is given in *pathname*, which is dereferenced if it is a symbolic link.\r\n\r\n[That link](https://man7.org/linux/man-pages/man2/chmod.2.html) is to a Linux manpage, but this holds across most, if not all, Unix-like systems. The macOS chmod(2) manpage is less explicit about this, but as noted in \"Steps to reproduce,\" I have verified this bug on macOS as well as GNU/Linux.\r\n\r\nIn contrast, `os.chmod` with `follow_symlink=False`, or `os.lchmod`, use lchmod(2), and do not deference symlinks. However, they are [not available on all systems](https://docs.python.org/3/library/os.html#os.supports_follow_symlinks). For example, Linux has no such system call.\r\n\r\nThis bug does not affect Windows. Although Windows [has symbolic links](https://blogs.windows.com/windowsdeveloper/2016/12/02/symlinks-windows-10/), `os.chmod` does not dereference them on Windows. See \"Conceptual examination\" below for details.\r\n\r\n### Steps to reproduce\r\n\r\n#### General approach\r\n\r\nAs detailed below in \"Why this `os.chmod` call only helps on Windows\", there are various scenarios in which `rmtree` will fail to delete a file on a Unix-like system, but the easiest to produce, which is also probably the most common in practice, is that the user lacks write permissions on the containing directory. If that directory contains a symbolic link to a file outside of it, then the failure to delete the directory will trigger an `os.chmod` affecting that out-of-tree file, in the callback `git.util.rmtree` passes to `shutil.rmtree`. So the steps, in brief, are:\r\n\r\n1. Create a directory, which will be passed to `git.util.rmtree`.\r\n2. Create a symbolic link in that directory, whose target is outside the directory. To observe the bug, the permissions of the target should *not* already be 0200, because that is what they will be changed to as a result of the bug.\r\n3. Use `chmod -w` on the directory.\r\n4. Pass the directory's name to `git.util.rmtree`.\r\n5. Observe that the permissions on the symbolic link's target have been changed, even though it is outside the directory being deleted.\r\n\r\n#### Script to reproduce the bug easily\r\n\r\nMore concretely, I have verified on Ubuntu 22.04.3 LTS and macOS 12.6.9 that the bug can be triggered by creating an executable script `perm.sh` with the contents (also [in this gist](https://gist.github.com/EliahKagan/b2b14bcf1620dda81c40f4bb540cf85a), for convenience):\r\n\r\n```sh\r\n#!/bin/sh\r\n\r\nset -e\r\n\r\nmkdir dir1\r\ntouch dir1/file\r\nchmod -w dir1/file\r\nprintf 'Permissions BEFORE rmtree call:\\n'\r\nls -l dir1/file\r\nprintf '\\n'\r\n\r\nmkdir dir2\r\nln -s ../dir1/file dir2/symlink\r\nchmod -w dir2\r\npython -c 'from git.util import rmtree; rmtree(\"dir2\")' || true\r\nprintf '\\nPermissions AFTER rmtree call:\\n'\r\nls -l dir1/file\r\n```\r\n\r\nThen run the script to see how attempting to delete `dir2` with `git.util.rmtree` changes the permissions of `dir1/file`, even though `dir1/file` is not inside `dir2`:\r\n\r\n```text\r\n$ ./perm.sh\r\nPermissions BEFORE rmtree call:\r\n-r--r--r-- 1 ek ek 0 Oct 30 05:13 dir1/file\r\n\r\nTraceback (most recent call last):\r\n  File \"/usr/lib/python3.12/shutil.py\", line 695, in _rmtree_safe_fd\r\n    os.unlink(entry.name, dir_fd=topfd)\r\nPermissionError: [Errno 13] Permission denied: 'symlink'\r\n\r\nDuring handling of the above exception, another exception occurred:\r\n\r\nTraceback (most recent call last):\r\n  File \"\u003cstring\u003e\", line 1, in \u003cmodule\u003e\r\n  File \"/home/ek/repos-wsl/GitPython/git/util.py\", line 212, in rmtree\r\n    shutil.rmtree(path, onexc=handler)\r\n  File \"/usr/lib/python3.12/shutil.py\", line 769, in rmtree\r\n    _rmtree_safe_fd(fd, path, onexc)\r\n  File \"/usr/lib/python3.12/shutil.py\", line 697, in _rmtree_safe_fd\r\n    onexc(os.unlink, fullname, err)\r\n  File \"/home/ek/repos-wsl/GitPython/git/util.py\", line 203, in handler\r\n    function(path)\r\nPermissionError: [Errno 13] Permission denied: 'dir2/symlink'\r\n\r\nPermissions AFTER rmtree call:\r\n--w------- 1 ek ek 0 Oct 30 05:13 dir1/file\r\n```\r\n\r\n`git.util.rmtree` still failed with a `PermissionError`, because the reason it can't delete `dir2/symlink` is due to the permissions on `dir2` itself, which it does not modify. But before it retries and fails, it has already changed the permissions of that symbolic link's target, `dir1/file`, from 0444 to 0200.\r\n\r\n### Conceptual examination\r\n\r\nThe above demonstrates the bug in GitPython itself, but I think it is also clarifying to see the behavior of `os.chmod` itself on various systems (or at least, I found this useful myself when investigating the bug).\r\n\r\n#### Unix-like systems - affected\r\n\r\nOn a Unix-like system (as above, I also tested this on Ubuntu 22.04.3 LTS and macOS 12.6.9), run:\r\n\r\n```text\r\n$ touch file\r\n$ chmod -w file\r\n$ ln -s file symlink\r\n$ ls -l\r\ntotal 0\r\n-r--r--r-- 1 ek ek 0 Nov 13 11:51 file\r\nlrwxrwxrwx 1 ek ek 4 Nov 13 11:52 symlink -\u003e file\r\n```\r\n\r\n```text\r\n$ python3.12 -c 'import os, stat; os.chmod(\"symlink\", stat.S_IWUSR)'\r\n$ ls -l\r\ntotal 0\r\n--w------- 1 ek ek 0 Nov 13 11:57 file\r\nlrwxrwxrwx 1 ek ek 4 Nov 13 11:57 symlink -\u003e file\r\n```\r\n\r\nThe core concept can alternatively be demonstrated, on the same systems, by the [chmod(3)](https://linux.die.net/man/3/chmod) command, which also uses [chmod(2)](https://linux.die.net/man/2/chmod), by replacing the second part above with:\r\n\r\n```text\r\n$ chmod 200 symlink\r\n$ ls -l\r\ntotal 0\r\n--w------- 1 ek ek 0 Nov 13 11:51 file\r\nlrwxrwxrwx 1 ek ek 4 Nov 13 11:52 symlink -\u003e file\r\n```\r\n\r\n(One would replace `python3.12` with one's actual Python command, if different. This is not limited to Python 3.12.)\r\n\r\n#### Windows - unaffected\r\n\r\nIn contrast, on Windows 10 (with [developer mode enabled](https://blogs.windows.com/windowsdeveloper/2016/12/02/symlinks-windows-10/)), in PowerShell 7:\r\n\r\n```text\r\n\u003e New-Item -Path file -ItemType File | Out-Null\r\n\u003e Set-ItemProperty -Path file -Name IsReadOnly -Value $true\r\n\u003e New-Item -Path symlink -ItemType SymbolicLink -Value file | Out-Null\r\n\u003e Get-ChildItem\r\n\r\n    Directory: C:\\Users\\ek\\tmp\r\n\r\nMode                 LastWriteTime         Length Name\r\n----                 -------------         ------ ----\r\n-ar--          11/13/2023 12:42 PM              0 file\r\nla---          11/13/2023 12:42 PM              0 symlink -\u003e file\r\n\r\n\u003e python3.12 -c 'import os, stat; os.chmod(\"symlink\", stat.S_IWUSR)'\r\n\u003e Get-ChildItem\r\n\r\n    Directory: C:\\Users\\ek\\tmp\r\n\r\nMode                 LastWriteTime         Length Name\r\n----                 -------------         ------ ----\r\n-ar--          11/13/2023 12:42 PM              0 file\r\nla---          11/13/2023 12:42 PM              0 symlink -\u003e file\r\n```\r\n\r\n(That is on a system where Python was installed from the Windows Store. If Python was installed from an .exe file downloaded from python.org, then one must usually replace `python3.12` with `py -3.12`. As on Unix-like systems, one may change the version number as well.)\r\n\r\nObserve that `file` is still read-only, even after operating on `symlink` with `os.chmod`. In contrast--though not shown above--changing `\"symlink\"` to `\"file\"` in the Python command does work, changing the displayed mode from `-ar--` to `-a---` (and allowing `Remove-Item` to delete the file).\r\n\r\n### Is this a vulnerability?\r\n\r\n\u003cdetails\u003e\u003csummary\u003e\u003cem\u003e\u003cstrong\u003eDisclosure considerations\u003c/strong\u003e (click to expand, if interested)\u003c/em\u003e\u003c/summary\u003e\r\n\u003cp\u003eMy interpretation of the \u003ca href=\"https://github.com/gitpython-developers/GitPython/blob/main/SECURITY.md\"\u003esecurity policy\u003c/a\u003e is that it does not express a preference against opening a public bug report like this one in this situation, \u003cem\u003ebut I would be pleased to receive feedback in this area\u003c/em\u003e. The specific factors I considered in deciding to report the bug this way were that \u003cem\u003e(a)\u003c/em\u003e it might be a security bug, but \u003cem\u003e(b)\u003c/em\u003e this project\u0026#39;s security policy does not request strict coordinated vulnerability disclosure, \u003cem\u003e(c)\u003c/em\u003e I am unaware of a likely method to exploit it, \u003cem\u003e(d)\u003c/em\u003e I have been, perhaps unwisely, developing and testing the fix on a public branch of my fork before thinking deeply about its security implications, so I may have, in effect, already disclosed the issue publicly, \u003cem\u003e(e)\u003c/em\u003e I believe my fix and tests are complete, and by reporting publicly I can offer them in a way that is either faster or much more convenient to review than if I disclosed by email and waited to open a PR, \u003cem\u003e(f)\u003c/em\u003e I may soon be unavailable to contribute for a few days or weeks, and if I take the time to research exploitability further, the effects of \u003cem\u003ed\u003c/em\u003e would be exacerbated and the benefits of \u003cem\u003ee\u003c/em\u003e diminished, and \u003cem\u003e(g)\u003c/em\u003e the note in CVE-2023-40590 suggests to me that the security policy does not intend to discourage public disclosure that is based on such factors.\u003c/p\u003e\r\n\u003c/details\u003e \r\n\r\nOutside of its tests, GitPython's only use of `git.util.rmtree` is to delete submodules. This sounds concerning, since cloning an untrusted repository, including its submodules, and doing git operations in it, without running code in it, should ideally be safe. Such a repository could certainly have arbitrary symbolic links in a submodule that are checked out into its working tree, including symlinks to absolute paths, or relative paths like `../..`, that would cause upward traversal. Calling `os.chmod` on such symlinks, in the way `git.util.rmtree` does, would change permissions on the target (if owned by the user running the process) to 0200, adding write permissions for the owning user, and removing all other permissions.\r\n\r\nThis is assuming, however, that a failure during `rmtree` occurs. As discussed below, I believe this is hard for an attacker to produce deliberately on any affected system. Without it, the callback, and thus its `os.chmod` call, is never run.\r\n\r\nFor the problem that write permissions are added, it is a major mitigation that they are added only for the owner. But this could nonetheless *contribute* to a loss of integrity if, for example, a non-robust script is employed that goes by access alone to mutate some files in a directory but not others.\r\n\r\nI believe a larger concern is actually the permissions that are *removed*. In particular, if the untrusted symlink is to a directory outside the working tree, the read permission needed to list the directory and the executable permission needed to traverse into the directory are removed. Therefore, if an attacker who does not already have access to the system but controls the contents of an untrusted repository that is locally cloned with submodules can cause `shutil.rmtree` to fail when called by `git.util.rmtree`, then the attacker can carry out a denial of service attack.\r\n\r\n**But I do not know of a way a malicious submodule author could deliberately bring this about**, because:\r\n\r\n- On Windows, where deletion fails if a file is open, I believe it is not rare for `git.util.rmtree` to fail to perform an operation and call its callback. But Windows is unaffected by this bug. I *believe* a failure of an operation attempted during `rmtree` is actually rare on Unix-like systems (outside of GitPython's own tests, some of which deliberately produce it). So simply waiting for it to happen may not be realistic.\r\n- Git tracks file modes, but only in a limited way, distinguishing between symlinks and regular files, and tracking executable permissions. If executable permissions are absent on a directory, it cannot be traversed and `rmtree` would fail, but the dangerous symlink would not be reached. More importantly, git does not track directories themselves, so the file modes it stores do not directly create directories that cannot be traversed.\r\n- If an attacker can trick the user into running a `chmod`, `chattr`, or `chflags` command, then it can easily be done. But running such commands from a malicious source is already unsafe with or without GitPython. It is probably easier for an attacker to fool a user into running them on a file inside a cloned repository than elsewhere, but then the attacker could urge the user to run `chmod` directly on the cloned symlink.\r\n\r\nA secondary concern is that this could lead to denial of service against an application that uses GitPython but intends to expose only some of the capabilities of the user account running it. At least [as I understand it](https://github.com/gitpython-developers/GitPython/pull/1644#issuecomment-1730983361), that was the chief concern in CVE-2023-41040 (#1638), which was considered a vulnerability. But in that case, there were specific, identifiable *git operations* an application could be requested to perform that would trigger an unexpected access to another part of the filesystem and resource exhaustion. In this case, I am unaware of such a condition. However, I admit such a problem might be discovered with further research.\r\n\r\nEven if GitPython's use of `git.util.rmtree` to remove submodules is not to be considered vulnerable, `git.util.rmtree` is public, being listed in `__all__`. So it may be (or have been) a reasonable design choice for an application to make direct use of it, which some applications may be doing in an inadvertently vulnerable way. This would not be limited to situations involving submodules.\r\n\r\n### Why this `os.chmod` call only helps on Windows\r\n\r\nOn Windows, attempting to change a file's permissions to be writable and retrying deletion of the file can help, because of how file permissions and deletion interact on Windows: attempting to delete a read-only file will fail on Windows, even if the user could delete the file if it were not read-only. The `os.chmod` call in `git.util.rmtree` appears to have been introduced specifically to handle that situation on Windows.\r\n\r\nIn contrast, on a Unix-like system, it probably can never help. On the one hand, the widespread belief that access to a file's containing directory, and never the file itself, is all that is relevant to the ability to delete a file on a Unix-like system... is actually *wrong* on some Unix-like systems. There are specific circumstances, on some Unix-like systems, where a user gaining write access to a file they cannot currently delete, with no other changes, would cause the user to be able to delete it. Nonetheless, the `os.chmod` call used in `git.util.rmtree` does not help in those situations either. Roughly speaking, on Unix-like systems:\r\n\r\n- If the filesystem is mounted read-only, then obviously the file cannot be removed regardless of its permissions.\r\n- If the user cannot traverse into the containing directory (due to lacking executable permissions on the directory or its ancestors), the user cannot delete the file. But a file that can't be traversed *to* is never arrived at in `shutil.rmtree` and therefore is not passed to the callback `git.util.rmtree` passes `shutil.rmtree`.\r\n- If the user lacks write access to the directory, the user cannot delete a file in it, and adding write permissions to the file will not help.\r\n- Some operating systems, including the most popular Unix-like operating systems (those based on Linux or Darwin), support additional attributes or other flags--separate from Unix permissions and access control lists--that can render a file \"immutable\" in such a way that causes deletion to fail. See [`chattr`](https://ss64.com/bash/chattr.html) and [`chflags`](https://ss64.com/osx/chflags.html). But this is separate from *permissions*, so adding write permissions to the file wouldn't help.\r\n- Otherwise, if the user has write access to the containing directory, and that directory does not have the [sticky bit](https://en.wikipedia.org/wiki/Sticky_bit) set or the user owns the directory, then the user can delete the file. This is the case whether or not the file is writable, so adding write permissions to the file does not help, but also is not attempted.\r\n- If, instead, the directory (which the user has write access to) *does* have the sticky bit set, then on most Unix-like systems, the user's ability to delete the file is contingent on file *ownership*. Typically, the user can delete the file if the user owns the directory or--more often relevant--owns the file. This allows users to create and delete their own files in locations like `/tmp` but not delete other users' files there. **But this is where things get tricky.** On a minority of Unix-like systems, including some that continue in significant use today such as Solaris (and illumos systems such as OpenIndiana), having write access to the file suffices as an alternative to owning it (see [chmod(2)](https://docs.oracle.com/cd/E23824_01/html/821-1463/chmod-2.html#REFMAN2chmod-2), as cited in the \"Solaris 11\" row of [this table](https://en.wikipedia.org/wiki/Sticky_bit#Usage)). I have verified this behavior on OpenIndiana 2023.10.\r\n\r\nThe reason the `os.chmod` call in `git.util.rmtree` is nonetheless ineffective at allowing such a file to be deleted is that setting `S_IWUSR` with `os.chmod` on a Unix-like system only confers write permissions *to the file's owner*. But owning the file is already sufficient to allow deletion when one has write access on the containing sticky directory.\r\n\r\nI only say \"probably\" because of the possibility that some system may facilitate expressing strange access restrictions that would make deleting a file one owns contingent on having write access to it on a Unix-like system. I don't think ACLs would express this, but perhaps something like AppArmor could be used to achieve it. If this were done, I don't think whoever did it would expect GitPython or any other software to take special advantage of it, and I would be inclined to think that would still not make the removal of that `os.chmod` call on any Unix-like system a breaking change in GitPython.\r\n\r\n### Alternative solutions\r\n\r\nThis last section covers why I believe skipping the `os.chmod` call when not running on Windows (as proposed in #1739) is better than the other possible solutions that I am aware of.\r\n\r\nI believe it would be worthwhile to skip the `os.chmod` call outside Windows even if this symlink-related bug were absent and the only problem with the call outside Windows were its ineffectiveness, because:\r\n\r\n- As detailed above, the likelihood that it is helping on any non-Windows system is very low.\r\n- Given that the `git.util.rmtree` docstring [explains the distinction](https://github.com/gitpython-developers/GitPython/blob/74cc671826122f14951de48e7027c58eacaabe25/git/util.py#L203-L205) between `shutil.rmtree` and `git.util.rmtree` in terms of Windows behavior, the `os.chmod` call does not appear to have been intended to have any important effect outside of Windows.\r\n\r\nHowever, even if having `git.util.rmtree` attempt to change file permissions on non-Windows systems were to be considered worthwhile, fixing that `os.chmod` call to do so safely would be nontrivial, because:\r\n\r\n- Calling [`os.chmod`](https://docs.python.org/3/library/os.html#os.chmod) with `follow_symlinks=False` is [not supported on all systems](https://docs.python.org/3/library/os.html#os.supports_follow_symlinks). This includes popular Unix-like systems. For example, `os.chmod in os.supports_follow_symlinks` evaluates to `False` on my Ubuntu system (and other Linux-based systems).\r\n- The [`os.lchmod`](https://docs.python.org/3/library/os.html#os.lchmod) function is likewise not present on all systems.\r\n- A similar situation exists with the `pathlib` alternatives [`Path.chmod`](https://docs.python.org/3/library/pathlib.html#pathlib.Path.chmod) with `follow_symlinks=False` and [`Path.lchmod`](https://docs.python.org/3/library/pathlib.html#pathlib.Path.lchmod). These are also not available on all currently supported versions of Python.\r\n- If I understand correctly, Unix-like systems on which `os.chmod` does not support `follow_symlinks` may not have any *atomic* way to change permissions without dereferencing a symlink. So checking and then performing the operation risks the file being replaced by a symlink at the last moment. Even if the risk is small, the reward, in this case, is surely smaller.\r\n\r\nThe other alternative that may seem appealing is to attempt to actually solve plausible causes of a `PermissionError` on a Unix-like system, by making the logic more expansive and complicated, changing write permissions on containing directories, adding executable permissions to directories during traversal to reach and delete files in them, unsetting immutable flags/attributes on systems that support those, and so forth. But:\r\n\r\n- `git.util.rmtree` doesn't seem to need to do this--as far as I can tell, the only known problems with it failing to delete files in actual GitPython use are on Windows. So based on complexity alone, it might not be worthwhile.\r\n- Getting it right is hard, and some ways of getting it wrong would be security bugs, due to added race conditions or otherwise.\r\n- File permissions and other metadata that can stymie file deletion exist for a reason, and in some uses a user may intend them to prevent deletion, possibly including of a file in a submodule's working tree. So at least in the absence of known real-world cases where `git.util.rmtree` is seen to benefit from working around them, it would be difficult to know that doing so would really be an overall improvement.\r\n\r\nSuch logic, if added, might *somewhat* resemble [`TemporaryDirectory._rmtree`](https://github.com/python/cpython/blob/3.12/Lib/tempfile.py#L875-L903), but while I think `TemporaryDirectory` with its cleanup logic is reasonable to use in GitPython's unit tests, I think any logic along those lines in `git.util.rmtree` might have to differ substantially from it to avoid security-related problems. I cite it not to recommend the technique, but instead as a rough guess at the *lower bound* of how much more code it might take to do it.","author":{"url":"https://github.com/EliahKagan","@type":"Person","name":"EliahKagan"},"datePublished":"2023-11-14T02:25:36.000Z","interactionStatistic":{"@type":"InteractionCounter","interactionType":"https://schema.org/CommentAction","userInteractionCount":4},"url":"https://github.com/1738/GitPython/issues/1738"}

route-pattern/_view_fragments/issues/show/:user_id/:repository/:id/issue_layout(.:format)
route-controllervoltron_issues_fragments
route-actionissue_layout
fetch-noncev2:c000657e-fcce-2070-fa34-a964aa289872
current-catalog-service-hash81bb79d38c15960b92d99bca9288a9108c7a47b18f2423d0f6438c5b7bcd2114
request-idD3D4:2F3A46:5262B7:72DF6D:6968CBA1
html-safe-nonce1d597a914a86ed108d4404e6e6d96dcb3434996a913269967725d43db2bd859c
visitor-payloadeyJyZWZlcnJlciI6IiIsInJlcXVlc3RfaWQiOiJEM0Q0OjJGM0E0Njo1MjYyQjc6NzJERjZEOjY5NjhDQkExIiwidmlzaXRvcl9pZCI6Ijg2NDIzNjE3MjkyNzg4MDA5NyIsInJlZ2lvbl9lZGdlIjoiaWFkIiwicmVnaW9uX3JlbmRlciI6ImlhZCJ9
visitor-hmac022598bd38eeebb2065e03d624554e7a9d171516811a39dd21ecb2315145b470
hovercard-subject-tagissue:1991862615
github-keyboard-shortcutsrepository,issues,copilot
google-site-verificationApib7-x98H0j5cPqHWwSMm6dNU4GmODRoqxLiDzdx9I
octolytics-urlhttps://collector.github.com/github/collect
analytics-location///voltron/issues_fragments/issue_layout
fb:app_id1401488693436528
apple-itunes-appapp-id=1477376905, app-argument=https://github.com/_view_fragments/issues/show/gitpython-developers/GitPython/1738/issue_layout
twitter:imagehttps://opengraph.githubassets.com/b8e33e77ea9342894a9510b4eed187fe5b63a97fcdd57b0d579419e01b592892/gitpython-developers/GitPython/issues/1738
twitter:cardsummary_large_image
og:imagehttps://opengraph.githubassets.com/b8e33e77ea9342894a9510b4eed187fe5b63a97fcdd57b0d579419e01b592892/gitpython-developers/GitPython/issues/1738
og:image:altOverview On Unix-like systems, git.util.rmtree will change permissions outside the tree being removed, under some circumstances. This occurs specifically with git.util.rmtree; it is not a flaw inhe...
og:image:width1200
og:image:height600
og:site_nameGitHub
og:typeobject
og:author:usernameEliahKagan
hostnamegithub.com
expected-hostnamegithub.com
Noneaf2d7af0cc84117fa10bf36808605ef68a335c9d8a804b9cdac55f8d77230b00
turbo-cache-controlno-preview
go-importgithub.com/gitpython-developers/GitPython git https://github.com/gitpython-developers/GitPython.git
octolytics-dimension-user_id503709
octolytics-dimension-user_logingitpython-developers
octolytics-dimension-repository_id1126087
octolytics-dimension-repository_nwogitpython-developers/GitPython
octolytics-dimension-repository_publictrue
octolytics-dimension-repository_is_forkfalse
octolytics-dimension-repository_network_root_id1126087
octolytics-dimension-repository_network_root_nwogitpython-developers/GitPython
turbo-body-classeslogged-out env-production page-responsive
disable-turbofalse
browser-stats-urlhttps://api.github.com/_private/browser/stats
browser-errors-urlhttps://api.github.com/_private/browser/errors
releasecc844ab6ee0198cc2e2c142dcb8a5c2a61d48743
ui-targetfull
theme-color#1e2327
color-schemelight dark

Links:

Skip to contenthttps://github.com/gitpython-developers/GitPython/issues/1738#start-of-content
https://github.com/
Sign in https://github.com/login?return_to=https%3A%2F%2Fgithub.com%2Fgitpython-developers%2FGitPython%2Fissues%2F1738
GitHub CopilotWrite better code with AIhttps://github.com/features/copilot
GitHub SparkBuild and deploy intelligent appshttps://github.com/features/spark
GitHub ModelsManage and compare promptshttps://github.com/features/models
MCP RegistryNewIntegrate external toolshttps://github.com/mcp
ActionsAutomate any workflowhttps://github.com/features/actions
CodespacesInstant dev environmentshttps://github.com/features/codespaces
IssuesPlan and track workhttps://github.com/features/issues
Code ReviewManage code changeshttps://github.com/features/code-review
GitHub Advanced SecurityFind and fix vulnerabilitieshttps://github.com/security/advanced-security
Code securitySecure your code as you buildhttps://github.com/security/advanced-security/code-security
Secret protectionStop leaks before they starthttps://github.com/security/advanced-security/secret-protection
Why GitHubhttps://github.com/why-github
Documentationhttps://docs.github.com
Bloghttps://github.blog
Changeloghttps://github.blog/changelog
Marketplacehttps://github.com/marketplace
View all featureshttps://github.com/features
Enterpriseshttps://github.com/enterprise
Small and medium teamshttps://github.com/team
Startupshttps://github.com/enterprise/startups
Nonprofitshttps://github.com/solutions/industry/nonprofits
App Modernizationhttps://github.com/solutions/use-case/app-modernization
DevSecOpshttps://github.com/solutions/use-case/devsecops
DevOpshttps://github.com/solutions/use-case/devops
CI/CDhttps://github.com/solutions/use-case/ci-cd
View all use caseshttps://github.com/solutions/use-case
Healthcarehttps://github.com/solutions/industry/healthcare
Financial serviceshttps://github.com/solutions/industry/financial-services
Manufacturinghttps://github.com/solutions/industry/manufacturing
Governmenthttps://github.com/solutions/industry/government
View all industrieshttps://github.com/solutions/industry
View all solutionshttps://github.com/solutions
AIhttps://github.com/resources/articles?topic=ai
Software Developmenthttps://github.com/resources/articles?topic=software-development
DevOpshttps://github.com/resources/articles?topic=devops
Securityhttps://github.com/resources/articles?topic=security
View all topicshttps://github.com/resources/articles
Customer storieshttps://github.com/customer-stories
Events & webinarshttps://github.com/resources/events
Ebooks & reportshttps://github.com/resources/whitepapers
Business insightshttps://github.com/solutions/executive-insights
GitHub Skillshttps://skills.github.com
Documentationhttps://docs.github.com
Customer supporthttps://support.github.com
Community forumhttps://github.com/orgs/community/discussions
Trust centerhttps://github.com/trust-center
Partnershttps://github.com/partners
GitHub SponsorsFund open source developershttps://github.com/sponsors
Security Labhttps://securitylab.github.com
Maintainer Communityhttps://maintainers.github.com
Acceleratorhttps://github.com/accelerator
Archive Programhttps://archiveprogram.github.com
Topicshttps://github.com/topics
Trendinghttps://github.com/trending
Collectionshttps://github.com/collections
Enterprise platformAI-powered developer platformhttps://github.com/enterprise
GitHub Advanced SecurityEnterprise-grade security featureshttps://github.com/security/advanced-security
Copilot for BusinessEnterprise-grade AI featureshttps://github.com/features/copilot/copilot-business
Premium SupportEnterprise-grade 24/7 supporthttps://github.com/premium-support
Pricinghttps://github.com/pricing
Search syntax tipshttps://docs.github.com/search-github/github-code-search/understanding-github-code-search-syntax
documentationhttps://docs.github.com/search-github/github-code-search/understanding-github-code-search-syntax
Sign in https://github.com/login?return_to=https%3A%2F%2Fgithub.com%2Fgitpython-developers%2FGitPython%2Fissues%2F1738
Sign up https://github.com/signup?ref_cta=Sign+up&ref_loc=header+logged+out&ref_page=%2F%3Cuser-name%3E%2F%3Crepo-name%3E%2Fvoltron%2Fissues_fragments%2Fissue_layout&source=header-repo&source_repo=gitpython-developers%2FGitPython
Reloadhttps://github.com/gitpython-developers/GitPython/issues/1738
Reloadhttps://github.com/gitpython-developers/GitPython/issues/1738
Reloadhttps://github.com/gitpython-developers/GitPython/issues/1738
gitpython-developers https://github.com/gitpython-developers
GitPythonhttps://github.com/gitpython-developers/GitPython
Please reload this pagehttps://github.com/gitpython-developers/GitPython/issues/1738
Notifications https://github.com/login?return_to=%2Fgitpython-developers%2FGitPython
Fork 964 https://github.com/login?return_to=%2Fgitpython-developers%2FGitPython
Star 5k https://github.com/login?return_to=%2Fgitpython-developers%2FGitPython
Code https://github.com/gitpython-developers/GitPython
Issues 169 https://github.com/gitpython-developers/GitPython/issues
Pull requests 8 https://github.com/gitpython-developers/GitPython/pulls
Discussions https://github.com/gitpython-developers/GitPython/discussions
Actions https://github.com/gitpython-developers/GitPython/actions
Security Uh oh! There was an error while loading. Please reload this page. https://github.com/gitpython-developers/GitPython/security
Please reload this pagehttps://github.com/gitpython-developers/GitPython/issues/1738
Insights https://github.com/gitpython-developers/GitPython/pulse
Code https://github.com/gitpython-developers/GitPython
Issues https://github.com/gitpython-developers/GitPython/issues
Pull requests https://github.com/gitpython-developers/GitPython/pulls
Discussions https://github.com/gitpython-developers/GitPython/discussions
Actions https://github.com/gitpython-developers/GitPython/actions
Security https://github.com/gitpython-developers/GitPython/security
Insights https://github.com/gitpython-developers/GitPython/pulse
New issuehttps://github.com/login?return_to=https://github.com/gitpython-developers/GitPython/issues/1738
New issuehttps://github.com/login?return_to=https://github.com/gitpython-developers/GitPython/issues/1738
#1739https://github.com/gitpython-developers/GitPython/pull/1739
git.util.rmtree can change permissions outside tree on Unix (chmod traverses symlinks)https://github.com/gitpython-developers/GitPython/issues/1738#top
#1739https://github.com/gitpython-developers/GitPython/pull/1739
acknowledgedhttps://github.com/gitpython-developers/GitPython/issues?q=state%3Aopen%20label%3A%22acknowledged%22
https://github.com/EliahKagan
https://github.com/EliahKagan
EliahKaganhttps://github.com/EliahKagan
on Nov 14, 2023https://github.com/gitpython-developers/GitPython/issues/1738#issue-1991862615
git.util.rmtreehttps://github.com/gitpython-developers/GitPython/blob/74cc671826122f14951de48e7027c58eacaabe25/git/util.py#L200-L225
shutil.rmtreehttps://docs.python.org/3/library/shutil.html#shutil.rmtree
callshttps://github.com/gitpython-developers/GitPython/blob/74cc671826122f14951de48e7027c58eacaabe25/git/util.py#L211
CWE-59https://cwe.mitre.org/data/definitions/59.html
race conditionshttps://docs.python.org/3/library/shutil.html#shutil.rmtree.avoids_symlink_attacks
#1739https://github.com/gitpython-developers/GitPython/pull/1739
GitPython/git/util.pyhttps://github.com/gitpython-developers/GitPython/blob/74cc671826122f14951de48e7027c58eacaabe25/git/util.py#L208-L214
74cc671https://github.com/gitpython-developers/GitPython/commit/74cc671826122f14951de48e7027c58eacaabe25
chmod(2)https://man7.org/linux/man-pages/man2/chmod.2.html
That linkhttps://man7.org/linux/man-pages/man2/chmod.2.html
not available on all systemshttps://docs.python.org/3/library/os.html#os.supports_follow_symlinks
has symbolic linkshttps://blogs.windows.com/windowsdeveloper/2016/12/02/symlinks-windows-10/
in this gisthttps://gist.github.com/EliahKagan/b2b14bcf1620dda81c40f4bb540cf85a
chmod(3)https://linux.die.net/man/3/chmod
chmod(2)https://linux.die.net/man/2/chmod
developer mode enabledhttps://blogs.windows.com/windowsdeveloper/2016/12/02/symlinks-windows-10/
security policyhttps://github.com/gitpython-developers/GitPython/blob/main/SECURITY.md
CVE-2023-40590https://github.com/advisories/GHSA-wfm5-v35h-vwf4
as I understand ithttps://github.com/gitpython-developers/GitPython/pull/1644#issuecomment-1730983361
CVE-2023-41040https://github.com/advisories/GHSA-cwvm-v4w8-q58c
#1638https://github.com/gitpython-developers/GitPython/issues/1638
chattrhttps://ss64.com/bash/chattr.html
chflagshttps://ss64.com/osx/chflags.html
sticky bithttps://en.wikipedia.org/wiki/Sticky_bit
chmod(2)https://docs.oracle.com/cd/E23824_01/html/821-1463/chmod-2.html#REFMAN2chmod-2
this tablehttps://en.wikipedia.org/wiki/Sticky_bit#Usage
#1739https://github.com/gitpython-developers/GitPython/pull/1739
explains the distinctionhttps://github.com/gitpython-developers/GitPython/blob/74cc671826122f14951de48e7027c58eacaabe25/git/util.py#L203-L205
os.chmodhttps://docs.python.org/3/library/os.html#os.chmod
not supported on all systemshttps://docs.python.org/3/library/os.html#os.supports_follow_symlinks
os.lchmodhttps://docs.python.org/3/library/os.html#os.lchmod
Path.chmodhttps://docs.python.org/3/library/pathlib.html#pathlib.Path.chmod
Path.lchmodhttps://docs.python.org/3/library/pathlib.html#pathlib.Path.lchmod
TemporaryDirectory._rmtreehttps://github.com/python/cpython/blob/3.12/Lib/tempfile.py#L875-L903
acknowledgedhttps://github.com/gitpython-developers/GitPython/issues?q=state%3Aopen%20label%3A%22acknowledged%22
https://github.com
Termshttps://docs.github.com/site-policy/github-terms/github-terms-of-service
Privacyhttps://docs.github.com/site-policy/privacy-policies/github-privacy-statement
Securityhttps://github.com/security
Statushttps://www.githubstatus.com/
Communityhttps://github.community/
Docshttps://docs.github.com/
Contacthttps://support.github.com?tags=dotcom-footer

Viewport: width=device-width


URLs of crawlers that visited me.