Re: maybe a bug in copy files

[Pavel Tsekov <ptsekov gmx net>]

Hello Egmont,

On Thu, 22 Feb 2007, Egmont Koblinger wrote:

> Hi,
>
> > [...] As it can be
> > seen the patch posted by Andrew calls chmod() on the target
> > file only if "preserve attributes" is set. However, it has to
> > be called in both cases since the destination file is created
> > with mode 600 initially due to security concerns - more info
> > can be found in file.c .
>
> I think this is overcomplicated... open() does not create the file with the
> permission taken from its third argument, it masks it with umask. So
> currently the file is not created with mode 600, but with mode (600 &
> ^umask).
>
> What's wrong with the following simple solution?
>
> When creating the file, pass the permissions of the old file to the open()
> call. This way it will have no more permissions than the original file, and
> no more permission than umask suggests.
>
> When copying is finished, call chmod() only if preserve attributes is set.

    /* Create the new regular file with small permissions initially,
       do not create a security hole.  FIXME: You have security hole
       here, btw. Imagine copying to /tmp and symlink attack :-( */

    while ((dest_desc = mc_open (dst_path, O_WRONLY | (ctx->do_append ?
            O_APPEND : (O_CREAT | O_TRUNC)), 0600)) < 0) {

I remember that there is a discussion somewhere on the mailing list
as to what this security concern is. I will try to dig it and see
whether it really makes sense to do it the way it currently is.