Re: maybe a bug in copy files

Hello Egmont,

On Thu, 22 Feb 2007, Egmont Koblinger wrote:


[...] 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 &

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.

[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]