bug#61386: [PATCH] cp,mv,install: Disable sparse copy on macOS

[Pádraig Brady]

On 15/02/2023 10:56, Paul Eggert wrote:
> Attached is an updated proposal for the fclonefileat patch.
>
> CVE-2021-30995 confirmed my suspicion that coreutils 9.1 and the current
> bleeding-edge coreutils on Savannah both have an exploitable security
> bug on macOS. Although I hope this patch fixes the bug, it could use
> more pairs of eyes, given that Apple had so many problems fixing its own
> security bug involving fclonefileat, and given that the coreutils code
> has so many flags and conditions.

The logic looks good to me, and all tests pass here on macOS 13.2

I see the added `if (! (cloned_mode & ~desired_mode))` restriction I presume
for security reasons, so that we only _add_ permission bits to those copied by 
fclonefileat(),
and never create an overly accessible file and then remove permission bits.
That wasn't obvious to me at least so I'd suggest adding comments from the diff 
below.

For the record this restriction will mean we can't reflink in some cases.
For example the following would fail with ENOSUP on macOS:

  touch file;
  chmod g+w file
  umask 0022
  cp --reflink=always file file.copy

Using -p in the above cp command would work though.

thanks,
Pádraig.

diff --git a/src/copy.c b/src/copy.c
index c5f2cc186..7c866e4fb 100644
--- a/src/copy.c
+++ b/src/copy.c
@@ -1284,6 +1284,7 @@ copy_reg (char const *src_name, char const *dst_name,
                : x->set_mode ? x->mode
                : ((x->explicit_no_preserve_mode ? MODE_RW_UGO : dst_mode)
                   & ~ cached_umask ()));
+          /* If fclonefileat() would not set undesired extra permissions.  */
           if (! (cloned_mode & ~desired_mode))
             {
               int fc_flags
@@ -1318,7 +1319,9 @@ copy_reg (char const *src_name, char const *dst_name,
                         }
                     }

+                  /* any desired permissions that weren't cloned.  */
                   extra_permissions = desired_mode & ~cloned_mode;
+
                   if (!extra_permissions
                       && (!x->preserve_mode || (fc_flags & CLONE_ACL)
                           || !fd_has_acl (source_desc)))