[vim/vim] vim_rename/vim_copyfile: mch_getperm failure (-1) is passed as O_CREAT mode without checking (Issue #20824)

[Shixin Tan]

tsx543 created an issue (vim/vim#20824)

Steps to reproduce

1. Build any normal Vim (path is portable src/fileio.c).
2. Trigger vim_rename() on a cross-filesystem rename so it falls through to vim_copyfile() after mch_rename() fails with EXDEV (common for :w / backup / swap paths that cross mounts).
3. Between the earlier successful mch_stat(from) in vim_rename() and the later mch_getperm(from) inside vim_copyfile(), make mch_getperm(from) fail (TOCTOU: remove/replace from, or a transient stat failure).
4. vim_copyfile() still calls mch_open(..., O_CREAT|O_EXCL|..., (int)perm) with perm == -1, so the creation mode becomes an all-ones / overly permissive mode (low bits typically 07777 before umask), instead of failing safely.
   Relevant code in current master (src/fileio.c):

/* vim_rename(): existence check, then rename; on failure: */
if (mch_rename((char *)from, (char *)to) == 0)
    return 0;
...
return vim_copyfile(from, to);

/* vim_copyfile(): */
perm = mch_getperm(from);
...
fd_out = mch_open((char *)to,
        O_CREAT|O_EXCL|O_WRONLY|O_EXTRA|O_NOFOLLOW, (int)perm);

There is no check that perm >= 0 (or that mch_getperm succeeded) before using it as the creation mode.

Expected behaviour

If mch_getperm(from) fails, vim_copyfile() / vim_rename() should abort with an error and must not create the destination with an unchecked / invalid mode derived from -1.

Version of Vim

9.2.0838

Environment

Operating system: Linux
Terminal: any
Value of $TERM: any
Shell: any

Logs and stack traces

—
Reply to this email directly, view it on GitHub, or unsubscribe.
Triage notifications, keep track of coding agent tasks and review pull requests on the go with GitHub Mobile for iOS and Android. Download it today!
You are receiving this because you are subscribed to this thread.Message ID: <vim/vim/issues/20824@github.com>