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.
_______________________________________________
Mc-devel mailing list
http://mail.gnome.org/mailman/listinfo/mc-devel
