Re: maybe a bug in copy files
On Mon, Feb 26, 2007 at 08:38:37PM +0200, Pavel Tsekov wrote: Ok. Maybe we are talking about different issues. What I was talking about was that the current code in MC requires the chmod() that i've ressurected. From what I understand you was talking that if we change the code we may avoid the chmod() call at the end. Is this right ? Yes, at least in the do not preserve permissions case we could avoid that chmod, and hence have a simplercleaner code. I hope. -- Egmont ___ Mc-devel mailing list http://mail.gnome.org/mailman/listinfo/mc-devel
Re: maybe a bug in copy files
On Thu, Feb 22, 2007 at 05:44:39PM +0200, Pavel Tsekov wrote: Here it is: http://mail.gnome.org/archives/mc-devel/2006-June/msg00063.html Shall we discuss how to create the file in a secure manner and avoid the call to mc_chmod() ? I think we're talking about two separate issues. One is how to safely create a file (without race condition) so that we really create _that_ file and not another one via a new symlink. The answer is O_EXCL. The other issue is how to create the file with the right permissions so that we don't leak information. I guess I proposed a cleaner solution for this than the current one. I can't see any correlation between these two issues. -- Egmont ___ Mc-devel mailing list http://mail.gnome.org/mailman/listinfo/mc-devel
Re: maybe a bug in copy files
Hello, On Mon, 26 Feb 2007, Egmont Koblinger wrote: On Thu, Feb 22, 2007 at 05:44:39PM +0200, Pavel Tsekov wrote: Here it is: http://mail.gnome.org/archives/mc-devel/2006-June/msg00063.html Shall we discuss how to create the file in a secure manner and avoid the call to mc_chmod() ? I think we're talking about two separate issues. One is how to safely create a file (without race condition) so that we really create _that_ file and not another one via a new symlink. The answer is O_EXCL. The other issue is how to create the file with the right permissions so that we don't leak information. I guess I proposed a cleaner solution for this than the current one. I can't see any correlation between these two issues. Ok. Maybe we are talking about different issues. What I was talking about was that the current code in MC requires the chmod() that i've ressurected. From what I understand you was talking that if we change the code we may avoid the chmod() call at the end. Is this right ? ___ Mc-devel mailing list http://mail.gnome.org/mailman/listinfo/mc-devel
Re: maybe a bug in copy files
Hello Lubomir, On Mon, 22 Jan 2007, Lubomir Grajciar wrote: When I try to copy file(s) with unchecked preserve atributes checkbox as regular user, copied files hasn't attributes based on my umask. My umask is: 0022 Copied file attributes: 600 When I try to create a new file (for example with shift-F4), it is created with right attributes (644). Thanks for noticing and reporting this! It is indeed a bug and it has been here for the last 5 years. For the curious - there is a thread with a subject of Problems with vfs / ftpfs started on 27 Feb 2002. 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'll revert that patch. ___ Mc-devel mailing list http://mail.gnome.org/mailman/listinfo/mc-devel
Re: maybe a bug in copy files
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. -- Egmont ___ Mc-devel mailing list http://mail.gnome.org/mailman/listinfo/mc-devel
Re: maybe a bug in copy files
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
Re: maybe a bug in copy files
Hello, On Thu, 22 Feb 2007, Pavel Tsekov wrote: 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. Here it is: http://mail.gnome.org/archives/mc-devel/2006-June/msg00063.html Shall we discuss how to create the file in a secure manner and avoid the call to mc_chmod() ? ___ Mc-devel mailing list http://mail.gnome.org/mailman/listinfo/mc-devel
