Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Problem in the Copy operation #2278

Open
mc-butler opened this issue Jul 12, 2010 · 23 comments
Open

Problem in the Copy operation #2278

mc-butler opened this issue Jul 12, 2010 · 23 comments
Assignees
Labels
area: core Issues not related to a specific subsystem prio: medium Has the potential to affect progress ver: 4.7.4 Reproducible in version 4.7.4

Comments

@mc-butler
Copy link

Important

This issue was migrated from Trac:

Origin https://midnight-commander.org/ticket/2278
Reporter eshkrig (eshkrig@….com)
Mentions gotar@….pl, howaboutsynergy@….me

Hi.
Sorry for my English.
Previously, copying a file does not change access permissions of the destination file if the check box "Preserve attributes" in the Copy window is not set.
For some time it is not so: if you uncheck the "Preserve attributes" then access permissions of the destination file are set in accordance with the value of umask, which can lead to information disclosure(i.e. security problem).

Note

Original attachments:

@mc-butler
Copy link
Author

Changed by eshkrig (eshkrig@….com) on Sep 11, 2010 at 7:42 UTC (comment 1)

  • Version changed from 4.7.3 to 4.7.4

The problem is still present in version 4.7.4

@mc-butler
Copy link
Author

Changed by eshkrig (eshkrig@….com) on Feb 20, 2011 at 15:56 UTC (comment 2)

the problem is still there:
app-misc/mc-4.7.5.1

@mc-butler
Copy link
Author

Changed by eshkrig (eshkrig@….com) on Apr 14, 2011 at 17:32 UTC (comment 3)

the problem is still there:
app-misc/mc-4.7.5.2

@mc-butler
Copy link
Author

Changed by andrew_b (@aborodin) on Apr 15, 2011 at 7:29 UTC

  • Description edited
  • Milestone changed from 4.7 to 4.8

Replying to eshkrig (#2278):

Previously, copying a file does not change access permissions of the destination file if the check box "Preserve attributes" in the Copy window is not set. For some time it is not so: if you uncheck the "Preserve attributes" then access permissions of the destination file are setted in accordance with the value of umask,

Yes, that done in #72. But how do you think should be a difference whether "Preserve attributes" is checked or not?

@mc-butler
Copy link
Author

Changed by eshkrig (eshkrig@….com) on Apr 16, 2011 at 7:42 UTC (comment 5)

IMHO: existing files should only get new content (as earlier versions do)

for example:
-rw------- /path_dst/dir1/secret_file # secret file with old contents
-rw-rw-r-- /path_dst/dir1/some_file

-rw------- /path_src/dir1/secret_file # secret file with new contents

when copying dir
/path_src/dir1 to /path_dst
we have got
-rw-r--r-- /path_dst/dir1/secret_file

That is fail becose structure may contain thousands of hundreds of thousands of catalogs.
And how about ACLs on /path_dst/dir1 ?

As I mention there is security bug for 9 months now (I am sad).

@mc-butler
Copy link
Author

Changed by andrew_b (@aborodin) on Apr 16, 2011 at 8:11 UTC (comment 5.6)

Replying to eshkrig:

IMHO: existing files should only get new content (as earlier versions do)

I don't quite understand, why you check "Preserve attributes" off if you want keep permissions.

@mc-butler
Copy link
Author

Changed by eshkrig (eshkrig@….com) on Apr 16, 2011 at 8:41 UTC (comment 6.7)

Replying to andrew_b:

I don't quite understand, why you check "Preserve attributes" off if you want keep permissions.

Because here changes the file owner and permissions.

For example as root I often have to modify configuration files in users' home directories.
Before, I could use the MC and the copy prepared files/directories to the user's home directory.

Now the situation is as follows:
If I copy the files with the "Preserve attributes" on - owner of the files is changed to root
If I copy the files with the "Preserve attributes" off - file permissions are changed to 644(-rw-r--r--) and this is not dependent on the established permissions to the source or the destination files

@mc-butler
Copy link
Author

Changed by eshkrig (eshkrig@….com) on Jul 14, 2011 at 18:57 UTC

@mc-butler
Copy link
Author

Changed by eshkrig (eshkrig@….com) on Jul 14, 2011 at 18:58 UTC (comment 8)

  • Branch state set to no branch

The patch, which fixes the problem

@mc-butler
Copy link
Author

Changed by bradatec (ps@….jikos.cz) on Sep 23, 2011 at 6:12 UTC (comment 9)

My bug was marked as a dupe of this (#2615) although it seems to me its something different.
In short, when copying files as a root, normal files get correct file/group id but it get wrong for soft links (preserve attributes are on).

@mc-butler
Copy link
Author

Changed by gotar (gotar@….pl) on Oct 10, 2011 at 10:38 UTC (comment 10)

  • Cc set to gotar@….pl

@mc-butler
Copy link
Author

Changed by slavazanko (@slavaz) on Mar 22, 2012 at 15:01 UTC (comment 11)

  • Owner set to slavazanko
  • Status changed from new to accepted

@mc-butler
Copy link
Author

Changed by slavazanko (@slavaz) on Mar 22, 2012 at 15:59 UTC (comment 12)

  • Branch state changed from no branch to on review

Created branch 2278_preserve_attrs_if_dest_exists [a58aee0]

Review, please.

@mc-butler
Copy link
Author

Changed by slavazanko (@slavaz) on Mar 22, 2012 at 19:03 UTC (comment 13)

  • Milestone changed from 4.8 to 4.8.3

@mc-butler
Copy link
Author

Changed by andrew_b (@aborodin) on Mar 25, 2012 at 14:21 UTC (comment 14)

  • Votes set to andrew_b

@mc-butler
Copy link
Author

Changed by angel_il (@ilia-maslakov) on Mar 26, 2012 at 6:16 UTC (comment 15)

  • Votes changed from andrew_b to andrew_b angel_il
  • Branch state changed from on review to approved

@mc-butler
Copy link
Author

Changed by slavazanko (@slavaz) on Mar 26, 2012 at 8:02 UTC (comment 16)

  • Keywords set to stable-candidate
  • Resolution set to fixed
  • Branch state changed from approved to merged
  • Status changed from accepted to testing
  • Votes changed from andrew_b angel_il to commited-master

Merged to master: [3907f19].

git log --pretty=oneline 68e1b4f..f0f39cb

@mc-butler
Copy link
Author

Changed by slavazanko (@slavaz) on Mar 26, 2012 at 8:07 UTC (comment 17)

Merged to stable: [d79db98].

git log --pretty=oneline 4fd3dee..82dcbe2

@mc-butler
Copy link
Author

Changed by slavazanko (@slavaz) on Mar 26, 2012 at 8:07 UTC (comment 18)

  • Votes changed from commited-master to commited-master commited-stable
  • Keywords stable-candidate deleted
  • Status changed from testing to closed

@mc-butler
Copy link
Author

Changed by eshkrig (eshkrig@….com) on Mar 26, 2012 at 16:34 UTC (comment 19)

Thank you.

@mc-butler
Copy link
Author

Changed by onkeltem (aneganov@….com) on Oct 11, 2012 at 18:05 UTC (comment 20)

  • Resolution fixed deleted
  • Status changed from closed to reopened

I'm not sure what exactly was fixed but I'm on 4.8.6-1 now and see that copy operation doesn't respect umask set on a remote host when using ssh.

So I connect to a remote via "Shell link...".

There I have umask set via pam_umask to 026.

If I copy a file/directory from local host to the remote and set Preserve Attributes to OFF, then the file/directory is created with 644/755 permissions.

Also I would like to have "Preserve attributes" OFF by default, any ideas how to do this?

UPDATE

After looking in code it becomes clear that MC takes local mask and use it to chmod'ing a remote object:

  else if (!dst_exists)
  {
    src_mode = umask (-1);
    umask (src_mode);
    src_mode = 0100666 & ~src_mode;
    mc_chmod (dst_vpath, (src_mode & ctx->umask_kill));
  }

https://github.com/MidnightCommander/mc/blob/master/src/filemanager/file.c#L1894

This happens with both SFTP and FISH VFSes which I tested.
The correct behavior would be to not chmod at all when we deal with a remote VFS. So instead of:

  else if (!dst_exists)

there should be something like:

  else if (!dst_exists && !<vfs_is_remote>)

Unfortunately I don't know how to check if a vfs is remote. Please fix this.

@mc-butler
Copy link
Author

Changed by andrew_b (@aborodin) on Sep 5, 2014 at 9:11 UTC (comment 21)

  • Milestone changed from 4.8.3 to Future Releases

@mc-butler
Copy link
Author

Changed by howaboutsynergy on May 6, 2019 at 9:19 UTC (comment 22)

  • Cc changed from gotar@….pl to gotar@….pl, howaboutsynergy@….me

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: core Issues not related to a specific subsystem prio: medium Has the potential to affect progress ver: 4.7.4 Reproducible in version 4.7.4
Development

No branches or pull requests

2 participants