Author Topic: Why CoreUtils/cp does need to fstat a file for a file-copy?  (Read 769 times)

0 Members and 1 Guest are viewing this topic.

Offline DiTBhoTopic starter

  • Super Contributor
  • ***
  • Posts: 3981
  • Country: gb
Why coreutils/cp does need to contain these lines?

Code: [Select]
   /*
    * Compare the source dev/ino from the open file to the incoming,
    * saved ones obtained via a previous call to stat
    */
  if (! SAME_INODE (*src_sb, src_open_sb))
    {
      error (0, 0,
             _("skipping file %s, as it was replaced while being copied"),
             quote (src_name));
      return_val = false;
      goto close_src_desc;
    }

Why a file-copy does need to "fstat" files and compare the source dev/ino from the open file to the incoming, saved ones obtained via a previous call to stat?!?

The result is that when you copy a file on some filesystems ... this happens

Code: [Select]
#cp guinea-pig-1.txt guinea-pig-2.txt
skip file 'guinea-pig-1.txt', since it was replaced during copying

/bin/cp returns FAILURE

This stuff gets really weird when copying a file, failing with a fancy error message.
CoreUtils/{ cp, mv } work OKay copying from local disk.
This only seems to happen when the source file is on an { NFS, CIFS/Samba } volume.
However, I also have this problem on my filesystem that I'm trying to integrate into GNU/Linux.

I still haven't understood why  :-//

I wrote a light version of these file-copy tools, much simpler than Coreutils/{ cp, mv }, but I would like a (toy) filesystem not to depend on custom "tools".
The opposite of courage is not cowardice, it is conformity. Even a dead fish can go with the flow
 

Offline ejeffrey

  • Super Contributor
  • ***
  • Posts: 3773
  • Country: us
Re: Why CoreUtils/cp does need to fstat a file for a file-copy?
« Reply #1 on: May 31, 2024, 03:31:31 am »
I think that shows up if you are copying multiple files into a directory and one of the source files is in the destination directory and is overwritten by a previously copied file.

Why cp skips the file in this case I'm not sure.  For this to come up with a posix compliant filesystem is almost certainly a mistake on the users part with carless use of wildcards or find expressions, so I guess reporting an error is fine, but it will cause problems on non-posix filesystems that can't keep the inode constant.

I understand why network filesystems cannot guarantee correct behavior under all conditions, but a local filesystem shouldnt have this problem.
 

Offline DiTBhoTopic starter

  • Super Contributor
  • ***
  • Posts: 3981
  • Country: gb
Re: Why CoreUtils/cp does need to fstat a file for a file-copy?
« Reply #2 on: May 31, 2024, 01:50:27 pm »
I still don't understand either. It looks weird at the moment.
But I know there are different ways to inplement a "file copy" tool
  • open(fd_rd, fd_wr), loop(EOF(fd_rd)| is_ok) { read(fd_rd), write(fd_wr) }, close(fd_rd, fd_wr), with all the read and write buffers in userspace. Simplest way to achieve it
  • mmap(), buffers work in kernel space
  • via sendfile() system call, so everything is done in kernel space. In theory it should only work with sockets, but it seems it has become a superset and also works with file descriptors
  • via copy_file_range() system call, so everything is done in kernel space. It gives better results, and it's the fastest way, only if the filesystem supports “copy-on-write”
  • via clonefile() + indirect ioctl system call, It seems very convoluted to me, but they say that it can give better results, again only if the filesystem supports “copy-on-write” and other modern mechanisms

In the past, I only implemented the first (simplest) one, because on XINU I don't have any of the other possibilities offered by the Linux kernel.
The opposite of courage is not cowardice, it is conformity. Even a dead fish can go with the flow
 

Offline DiTBhoTopic starter

  • Super Contributor
  • ***
  • Posts: 3981
  • Country: gb
Re: Why CoreUtils/cp does need to fstat a file for a file-copy?
« Reply #3 on: May 31, 2024, 02:10:09 pm »
Funny and somehow useful alternatives to "cp"
Code: [Select]
name         description                                          written in...
--------------------------------------------------------------------------------
coreutils    Default cp, mv                                       C
xcp          Extended cp                                          Rust
fcp          Significantly faster alternative to cp               Rust
gcp          Goffi's cp, a fancy file copier                      Python
pycp         cp and mv with a progressbar                         Python
rsync        fast incremental file transfer                       C
fuc          Fast UNIX cmds, provides alternatives for rm and cp  Rust
renameutils  Make copying files faster and less cumbersome        C

« Last Edit: May 31, 2024, 02:13:22 pm by DiTBho »
The opposite of courage is not cowardice, it is conformity. Even a dead fish can go with the flow
 

Offline Nominal Animal

  • Super Contributor
  • ***
  • Posts: 6462
  • Country: fi
    • My home page and email address
Re: Why CoreUtils/cp does need to fstat a file for a file-copy?
« Reply #4 on: May 31, 2024, 02:41:43 pm »
Why a file-copy does need to "fstat" files and compare the source dev/ino from the open file to the incoming, saved ones obtained via a previous call to stat?!?
Because TOCTOU: otherwise leaving a bait-and-switch race window open, exploitable by a local (unprivileged) attacker when a privileged user is copying an unprivileged file to a privileged location.

Fundamentally, there may be many more file names specified as command line parameters than can be kept open at the same time.  All source files need to be opened or stat()ed, at minimum to check if the arguments are files or directories, but also to avoid overwriting same files and detecting other problematic situations.  Thus, cp ends up stat()ing each source file twice: once beforehand, then again after opening it.  If the two differ, the source file was replaced between the two operations, and copying it could allow a security exploit.

A "bait-and-switch" attack relies on the target program first verifying the (unprivileged) source file, then the attacker changing it, before the target file uses (here, copies) it to a privileged directory.  This is a serious problem in general, and an endless source of exploitable time-of-check to time-of-use bugs.
 
The following users thanked this post: golden_labels, DiTBho

Offline ejeffrey

  • Super Contributor
  • ***
  • Posts: 3773
  • Country: us
Re: Why CoreUtils/cp does need to fstat a file for a file-copy?
« Reply #5 on: June 01, 2024, 03:05:37 am »
Its not really a good solution to that problem though, since cp doesn't verify that the first stat call gets the correct file.  And it doesn't check if the file contents are changed, only that it hasnt been deleted/moved and another file moved in place. 

The race condition is actually between the administrator deciding to copy a specific file and the file being copied.  cp only has visibility into a small part of that.

The only real solution is to copy the file to a privileged temporary location the have the administrator verify it's the correct file, then move it into place.
 

Offline DiTBhoTopic starter

  • Super Contributor
  • ***
  • Posts: 3981
  • Country: gb
Re: Why CoreUtils/cp does need to fstat a file for a file-copy?
« Reply #6 on: June 01, 2024, 05:18:39 am »
Time-of-check_to_time-of-use, TOCTOU

is it a problem when an application is under ptrace or secomp?

ptrace is very intrusive and injects a lot of breaks (sys_brk), I've never asked myself the problem, but now that I think about it I'm afraid it might alter the TOCTOU.
secomp is a mess, glibc doesn't even have a correct wrapper for it, and it's actually only supported since kernel 4.*, but it should be less intrusive. But I am not sure
« Last Edit: June 01, 2024, 12:51:13 pm by DiTBho »
The opposite of courage is not cowardice, it is conformity. Even a dead fish can go with the flow
 

Offline Nominal Animal

  • Super Contributor
  • ***
  • Posts: 6462
  • Country: fi
    • My home page and email address
Re: Why CoreUtils/cp does need to fstat a file for a file-copy?
« Reply #7 on: June 01, 2024, 08:26:38 am »
Its not really a good solution to that problem though
It does effectively ensure the inode is the same it based its operating decisions on, thus effectively closing the race window between time-of-check and time-of-use at the inode level.

If the inode remains the same, it is up to the user to worry about the contents being valid.
(There are many different ways to do that, so it's not up to cp to worry about that.)

The core argument is similar to why one should use nftw() instead of home-grown opendir()/readdir()/closedir() loops to traverse directory trees: nftw() implementations are supposed to use a similar mechanism (remembering inode details) to avoid reporting the same inode twice.  Most home-grown directory tree traversals handle renamed files and directories, and moving (parts) of the directory tree in/out during traversal, wrong.  Yet, nftw() will not abort when it detects that, and only tries to do its best.

is it problem when an application is under ptrace or secomp?
They can only widen an existing race window, not create a new one, for normal applications like cp.

When already ptraced, executing a new binary to elevate privileges (i.e., being setuid or with file capabilities) fails to acquire those privileges.  Thus, for applications and utilities that acquire new privileges via the filesystem, setuid or capabilities, will not be affected.
Applications that use a service that manages their privileges via process ID –– polkit and sudo replacements that reject Unix minimalist modular approaches for a framework/service based one ––, however, are vulnerable to manipulation via ptrace and seccomp.

The attack scenario is creating a tracee initially running at attackers credentials, allowing the tracing, setting suitable seccomp filters to hide it, then executing an application that elevates its privileges; after which the original tracer may be in control of a privileged process.  Again, this will not work if the privileges are elevated due to filesystem capabilities or setuid/setgid flags: the kernel will not elevate privileges for a ptraced process then.  Because seccomp allows such an attack to be hidden from the traced process, anything that uses an external process elevating the privileges based on process ID, will be susceptible.
 

Offline DiTBhoTopic starter

  • Super Contributor
  • ***
  • Posts: 3981
  • Country: gb
Re: Why CoreUtils/cp does need to fstat a file for a file-copy?
« Reply #8 on: June 01, 2024, 02:41:34 pm »
just verified, modern coreutils/cp uses the "sys_copy_file_range" syscal  :o :o :o
The opposite of courage is not cowardice, it is conformity. Even a dead fish can go with the flow
 

Offline Nominal Animal

  • Super Contributor
  • ***
  • Posts: 6462
  • Country: fi
    • My home page and email address
Re: Why CoreUtils/cp does need to fstat a file for a file-copy?
« Reply #9 on: June 01, 2024, 03:12:19 pm »
just verified, modern coreutils/cp uses the "sys_copy_file_range" syscal  :o :o :o
That is only after the source and target file descriptors have been opened, though; well after the stat() checks.

And it makes a lot of sense to use fs/read_write.c:sys_copy_file_range() (look for SYSCALL_DEFINE6(copy_file_range,), as more and more filesystems support the internal methods it uses.

The main benefit is that doing e.g. cp a b on an NFS filesystem, none of the file contents are transferred from the NFS server to the client; the entire copy operation is done on the server side instead.  On local filesystems, it can simplify to a metadata change at best; at worst, it still saves having to transfer the data over the kernel-userspace boundary, and makes maximal use of the page cache.  Really, no downsides that I can tell, except for added kernel-side complexity.
 

Offline ejeffrey

  • Super Contributor
  • ***
  • Posts: 3773
  • Country: us
Re: Why CoreUtils/cp does need to fstat a file for a file-copy?
« Reply #10 on: June 01, 2024, 03:51:51 pm »
Its not really a good solution to that problem though
It does effectively ensure the inode is the same it based its operating decisions on, thus effectively closing the race window between time-of-check and time-of-use at the inode level.

But what checks / decisions does cp make based on the initial stat call?  As far as I can tell cp doesn't have any way to check that the initial stat call resolved to the "correct" file.
 
The check shouldn't hurt of course. If the file changes during execution something is not right.  And it can reduce the window of vulnerability for some attacks, but not to zero.  It's just sort of intrinsically unsafe for an administrator to manipulate files in a world writeable location while other users are active.
 

Offline Nominal Animal

  • Super Contributor
  • ***
  • Posts: 6462
  • Country: fi
    • My home page and email address
Re: Why CoreUtils/cp does need to fstat a file for a file-copy?
« Reply #11 on: June 01, 2024, 06:07:19 pm »
As far as I can tell cp doesn't have any way to check that the initial stat call resolved to the "correct" file.
It definitely resolves to the inode named by the command line argument.  Thus, it cannot be "incorrect".

Put another way, if it opened each input file one at a time, fstat()ed them, and either read their entire contents into memory, or kept that same descriptor open, before writing any of the output files, then a single fstat() would suffice.  Because it can run out of file descriptors, it instead checks each input file first, decides what to do, and then re-opens each input file when it is ready to do the copy.  The second fstat() is necessary to ensure the inode that was opened is the same as it initially stat()ed/fstat()ed.

But what checks / decisions does cp make based on the initial stat call?
Instead of me making some assertions, let's examine that.  (Besides, current coreutils-9.5 is quite different to discussed above.)

Github has a mirror of the coreutils sources here, with the main cp source file being src/cp.c.  Note that those links default to the development head, so you might wish to select a specific tag.  The current newest release version is 9.5, and its src/cp.c is here.  (You'll also want to look in gl/lib/ for additional files.)  Its main() handles command-line options, with actual argument examination and copying done in src/cp.c:do_copy().

The first stat done is on the very last argument, using gl/lib/targetdir.c:target_directory_operand().  This is needed to verify that the last argument specifies a target directory for the source files.

If there is a target directory, each source file name is used to construct the target file name, and src/copy.c:copy() is called to copy that file.  It in turn uses src/copy.c:copy_internal() to do the actual work.  That in turn uses src/copy.c:follow_fstatat() to stat the file, to make sure it is not a directory.  If there are more than two arguments, x->src_info is set, and the stat info is added to the hash table using gnulib:lib/file-set.c:record_file().  Here, previously seen stat's are also used to detect the case where the same source file appears more than once via gnulib:lib/file-set.c:seen_file().  Note that these use both the file name and the stat info block, not just the stat info, to detect a repeated source file.

Progressing through that file, we find an use of src/copy.c:same_file_ok().  Its comment explains the reasons for its use: when the target file in the target directory already exists, it ensures it is not the same inode as the source file, because trying to copy the source to the target would destroy the source file.  (This can happen with for example a bind mount.)  You'll see related code that allows cp to copy a file over a symlink that points to the original file, for example.
Note, however, that this compares the source file to the tentative destination file.

Later on, we see two additional gnulib:lib/file-set.c:seen_file() checks.  (The third case, first one in that function, was to detect a repeated source file.)  The first of these detects the case where a later copy would overwrite an earlier target file, as would happen if you had
    rm -rf a b c ; mkdir a b c ; touch a/f b/f
and then did
    cp a/f b/f c
The result is that c/f will be a copy of a/f, and cp will display an error: cp: will not overwrite just-created 'c/f' with 'b/f'.

The second case checks that if an earlier copy caused a symlink to be created as a destination, that symlink will not be followed during creating further destination copies.  This, too, involves checking a destination file against source files.

When above checks have all passed, and the source file is a regular file, the actual copy is done by the src/copy.c:copy_reg() function.  This is where the current equivalent of the check mentioned in the thread starter post is, lines 1265-1274, via psame_inode() that is just a wrapper around the gnulib:lib/same-inode.h:PSAME_INODE() macro.

This is how the copy_reg() function verifies that the source file, after opening, is the same one the code used follow_fstatat() on.

If we look at all the decisions above, if all the code did was to support copying regular files, then it could have just opened the source file (some kind of follow_openat() followed by fstat() on the open descriptor), yes.  But the same code is actually used in cp, mv, and install, ensuring they all use the same logic (well, code!) for resolving source and target file names, and it supports quite a lot of options from creating symlinks or hardlinks to copying entire directory trees.

So, for regular files, the first stat() (filename-based follow_fstatat()) is to determine if the source file is a file or directory, that it isn't the same inode as its corresponding copy is (in which case copying would destroy the source file), and helps (along with filename) to detect duplicate source files.  The second fstat() after opening the source file "only" checks that it matches the one follow_fstatat() resolved to.  Technically, it could have opened the file from the get go, but because coreutils shares this part across multiple utilities and there are many options that change the behaviour, it does not.

Is it correct to say the two stats are needed to avoid TOCTOU?  I think so, yes.
One can argue that it is because of code complexity instead (because just opening the source file once, and keeping that descriptor open, would mean one fstat() call would suffice).  Or that it is there to ensure follow_fstatat() resolves to the same file the later open() does.

To me, the "verify still same" is a pattern I always apply to avoid TOCTOU, even if not security sensitive, when I cannot sensibly keep a descriptor or similar kernel-vetted handle open from the get go; and that not doing so is a bug.  Am I wrong?  I could be, but I don't think so.  I do have some practical experience in security-sensitive systems programming and oddball exploits and attacks, and consider even Apache SuEXEC unsafe (for reasons I've described here and here); if it matters to you, you can start with those and compare to what other security specialists say on such matters.

The check shouldn't hurt of course. If the file changes during execution something is not right.  And it can reduce the window of vulnerability for some attacks, but not to zero.  It's just sort of intrinsically unsafe for an administrator to manipulate files in a world writeable location while other users are active.
Oh, I definitely agree.  However, on the kinds of systems I've worked with, there are many different administrators and levels of "administrator".

The typical TOCTOU the above check detects is not an intentional attack, but an accident when two or more persons work on the same files at the same time.  Or when a human user accidentally interferes with a maintenance script copying some files, perhaps.

I prefer to use group-based access policies, because that way the "last owner user" is recorded as the file owner.  (I provide some additional tools to change ownership between users belonging to that group for transferring "ownership".  It's more like "who admits to being responsible for it", really.  In Linux, for local files (not NFS or other shared volumes), one can use file leases to detect concurrent editing attempts, where the file to be modified/edited is copied to a temporary no-access-to-other-users directory.  Not so much for "attacks" per se, but to avoid confusion due to concurrent edits.)

Thus, a typical scenario might be in some configuration directory only "site admins" or above can access, but two "site admins" making conflicting operations at the same time.  An example that comes to mind is when one admin starts to prep a new vhost or subsite on some web server, pops for refilling their coffee/tea/water, and another one notices in the mean time that the vhost/subsite directory tree has been created but not copied to yet, so starts doing that to be helpful.  In the mean time, the original person comes back and notices they forgot to press Enter to copy the contents to it.
("Yea, but that's the reason we have ticketing systems!" – except when you're training someone and what you said and what you did is not in perfect sync what they see. And so on. Accidents always happen.)
 


Share me

Digg  Facebook  SlashDot  Delicious  Technorati  Twitter  Google  Yahoo
Smf