From 7a923efb1d23971f04f336a019775aaaf803af2f Mon Sep 17 00:00:00 2001
From: Christian Kellner <christian@kellner.me>
Date: Wed, 1 Dec 2021 16:20:16 +0000
Subject: [PATCH] util/rmrf: handle broken symlinks

The current implementation of `rmtree` will try to fix permissions
when it encounters permission errors during its operation. This is
done by opening the target via `os.open` and then adjusting the
immutable flag and the permission bits. This is a problem when the
target is a broken symlink since open will fail with `ENOENT`. A
simple reproducer of this scenario is:
  $ mkdir subdir
  $ ln -s foo subdir/broken
  $ chmod a-w subdir/
  $ python3 -c 'import osbuild; osbuild.util.rmrf.rmtree("subdir")'

Since subdir is not writable, removing `subdir/broken` will fail
with `EPERM` and the `on_error` callback will try to fix it by
invoking `fixperms` on `subdir/broken` which will in `open` since
the target does not exist (broken symlink).

This is fixed by using `O_NOFOLLOW` to open so we will never open
the target. Instead `open` will fail with `ELOOP`; we ignore that
error and in fact we ignore now all errors from `open` since it
does not matter: if fixing the permissions didn't work `unlink`
will just fail (again) with `EPERM` and for symlinks it actually
doesn't matter since "on Linux the permissions of an ordinary
symbolic link are not used in an operations", see symlinks(7).
---
 osbuild/util/rmrf.py | 14 +++++++++++++-
 1 file changed, 13 insertions(+), 1 deletion(-)

diff --git a/osbuild/util/rmrf.py b/osbuild/util/rmrf.py
index d856904e..7a1c015e 100644
--- a/osbuild/util/rmrf.py
+++ b/osbuild/util/rmrf.py
@@ -55,7 +55,19 @@ def rmtree(path: str):
     def fixperms(p):
         fd = None
         try:
-            fd = os.open(p, os.O_RDONLY)
+
+            # if we can't open the file, we just return and let the unlink
+            # fail (again) with `EPERM`.
+            # A notable case of why open would fail is symlinks; since we
+            # want the symlink and not the target we pass the `O_NOFOLLOW`
+            # flag, but this will result in `ELOOP`, thus we never change
+            # symlinks. This should be fine though since "on Linux, the
+            # permissions of an ordinary symbolic link are not used in any
+            # operations"; see symlinks(7).
+            try:
+                fd = os.open(p, os.O_RDONLY | os.O_NOFOLLOW)
+            except OSError:
+                return
 
             # The root-only immutable flag prevents files from being unlinked
             # or modified. Clear it, so we can unlink the file-system tree.