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