mv: cross-device leaf chowns by path, contradicting the comment above it (GNU uses fchown)
src/uu/mv/src/mv.rs:1466 documents a guarantee the code does not fully provide:
// Open src/dst with O_NOFOLLOW and keep the fds alive across copy,
// chown, xattr, and chmod so a concurrent path-swap can't redirect any
// step to a different inode.Copy, xattr and chmod do honour it. chown does not. preserve_ownership (mv.rs:1527) takes two &Path, re-symlink_metadata()s them, and calls wrap_chown(to, …) (:1544) — a path-based lchown, not an operation on the dst_file fd that is open and in scope right there.
Line references are against main at be00b4c4e.
Trace
Cross-device move as root, source owned by uid 1000:
uutils: open("…/dst/k", O_WRONLY|O_CREAT|O_TRUNC|O_NOFOLLOW|O_CLOEXEC, 0600) = 4
lchown("…/dst/k", 1000, 1000) <- path-based
fchmod(4, 0664) <- fd-based
GNU: openat(AT_FDCWD, "…/dst/k2", O_WRONLY|O_CREAT|O_EXCL, 0600) = 4
fchown(4, 1000, 1000) <- fd-based
fchmod(4, 0100664)Control: GNU coreutils 9.11.130 (master).
Two divergences: GNU chowns the descriptor, and GNU's create carries O_EXCL where create_dest_restrictive(to, nofollow = true) (mv.rs:1481) does not.
Suggested fix
preserve_ownershipshould take the already-open&Fileand userustix::fs::fchown();- add
O_EXCLto themvdestination create; - correct the comment at
mv.rs:1466— as written it asserts a guarantee that does not hold, which is the part most likely to stop the next reviewer from looking.
Reported by @RelunSec (InsiteTech.jp). Triaged as a correctness/robustness bug: no primitive was found that reaches a file the attacker does not already control (lchown does not follow symlinks; fs.protected_hardlinks=1 blocks the hard-link route), but the divergence from GNU is real and the false comment should go.
Source: uutils/coreutils