GNU bug report logs - #3712
23.1.50; SECURITY: Tramp creates -rwxrwxrwx permission files with /su and /sudo method

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 3712 in the body.
You can then email your comments to 3712 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> emacsbugs.donarmstrong.com (full text, mbox):

From: Teemu Likonen <tlikonen <at> iki.fi>
To: emacs-pretest-bug <at> gnu.org
Subject: 23.1.50; SECURITY: Tramp creates -rwxrwxrwx permission files with /su and /sudo method
Date: Mon, 29 Jun 2009 18:16:30 +0300

When method /su: or /sudo: is used to _create_ a file the file's
permission will be set to -rwxrwxrwx (777), that is, allow everything
for everyone. Obviously this is serious security bug. Steps to
reproduce:

 1. Start Emacs as a normal user:

        emacs -Q

 2. Create a file in a directory to which the user who launched this
    Emacs session doesn't have write access.

        C-x C-f /su::/root/test.txt

 3. Write some content to the file and save it with "C-x C-s".

 4. Check file's permissions. It has 777 permission bits:

        $ ls -l /root/test.txt
        -rwxrwxrwx 1 root root 5 2009-06-29 17:58 /root/test.txt

For some reason, if I create similar file to the same user's home
directory who launched this Emacs session (/su::$HOME/test.txt) then it
gets 644 permissions (probably honoring umask settings).


In GNU Emacs 23.1.50.4 (i686-pc-linux-gnu, GTK+ Version 2.12.12)
 of 2009-06-29 on mithlond
Windowing system distributor `The X.Org Foundation', version 11.0.10402000
configured using `configure  '--prefix=/home/dtw/local''

Message #14 received at 3712 <at> emacsbugs.donarmstrong.com (full text, mbox):

From: Teemu Likonen <tlikonen <at> iki.fi>
To: 3712 <at> debbugs.gnu.org
Subject: Re: 23.1.50; SECURITY: Tramp creates -rwxrwxrwx permission files with /su and /sudo method
Date: Mon, 29 Jun 2009 22:10:59 +0300

On 2009-06-29 18:16 (+0300), Teemu Likonen wrote:

> When method /su: or /sudo: is used to _create_ a file the file's
> permission will be set to -rwxrwxrwx (777), [...]

This also happens when _editing_ an existing file because "backup by
renaming" will move the old file aside and the new version of file is
really creating a new file.

So, if you want to give your /etc/passwd and /etc/shadow the -rwxrwxrwx
permissions just edit the files with tramp's /su or /sudo method while
having backup by renaming enabled.

Message #19 received at 3712 <at> emacsbugs.donarmstrong.com (full text, mbox):

From: Michael Albinus <michael.albinus <at> gmx.de>
To: Teemu Likonen <tlikonen <at> iki.fi>
Cc: 3712 <at> debbugs.gnu.org
Subject: Re: bug#3712: 23.1.50; SECURITY: Tramp creates -rwxrwxrwx permission files with /su and /sudo method
Date: Mon, 29 Jun 2009 23:15:19 +0200

Teemu Likonen <tlikonen <at> iki.fi> writes:

> On 2009-06-29 18:16 (+0300), Teemu Likonen wrote:
>
>> When method /su: or /sudo: is used to _create_ a file the file's
>> permission will be set to -rwxrwxrwx (777), [...]

I've committed a fix, to both the trunk and the 23.1 branch.

> This also happens when _editing_ an existing file because "backup by
> renaming" will move the old file aside and the new version of file is
> really creating a new file.
>
> So, if you want to give your /etc/passwd and /etc/shadow the -rwxrwxrwx
> permissions just edit the files with tramp's /su or /sudo method while
> having backup by renaming enabled.

This I cannot reproduce. I have set `backup-by-copying' to nil. Backups
of files under /sudo::... have the same permissions as the original
file.

Best regards, Michael.

Message #24 received at 3712 <at> emacsbugs.donarmstrong.com (full text, mbox):

From: Teemu Likonen <tlikonen <at> iki.fi>
To: Michael Albinus <michael.albinus <at> gmx.de>
Cc: 3712 <at> debbugs.gnu.org
Subject: Re: bug#3712: 23.1.50; SECURITY: Tramp creates -rwxrwxrwx permission files with /su and /sudo method
Date: Tue, 30 Jun 2009 01:01:21 +0300

On 2009-06-29 23:15 (+0200), Michael Albinus wrote:

>> On 2009-06-29 18:16 (+0300), Teemu Likonen wrote:
>>> When method /su: or /sudo: is used to _create_ a file the file's
>>> permission will be set to -rwxrwxrwx (777), [...]
>
> I've committed a fix, to both the trunk and the 23.1 branch.

Thanks. Otherwise OK but I don't like the fact that it gives executable
bits (-rwxr-xr-x) by default. Normal behavior for new files is to drop
umask bits _and_ executable bits. Executable must be added explicitly.

Message #29 received at 3712 <at> emacsbugs.donarmstrong.com (full text, mbox):

From: Teemu Likonen <tlikonen <at> iki.fi>
To: 3712 <at> debbugs.gnu.org
Cc: Michael Albinus <michael.albinus <at> gmx.de>
Subject: Re: bug#3712: 23.1.50; SECURITY: Tramp creates -rwxrwxrwx permission files with /su and /sudo method
Date: Tue, 30 Jun 2009 01:31:50 +0300

On 2009-06-30 01:01 (+0300), Teemu Likonen wrote:

> On 2009-06-29 23:15 (+0200), Michael Albinus wrote:
>
>>> On 2009-06-29 18:16 (+0300), Teemu Likonen wrote:
>>>> When method /su: or /sudo: is used to _create_ a file the file's
>>>> permission will be set to -rwxrwxrwx (777), [...]
>>
>> I've committed a fix, to both the trunk and the 23.1 branch.
>
> Thanks. Otherwise OK but I don't like the fact that it gives executable
> bits (-rwxr-xr-x) by default. Normal behavior for new files is to drop
> umask bits _and_ executable bits. Executable must be added explicitly.

And when editing existing files it should obviously respect the bits
that the file already has. Currently -- even with this fix -- tramp is
adding "x" bits at some point because "backup by rename" moves old
version out of the way and new is created with -rwxr-xr-x bits.

Message #34 received at 3712 <at> emacsbugs.donarmstrong.com (full text, mbox):

From: Michael Albinus <michael.albinus <at> gmx.de>
To: Teemu Likonen <tlikonen <at> iki.fi>
Cc: 3712 <at> debbugs.gnu.org
Subject: Re: bug#3712: 23.1.50; SECURITY: Tramp creates -rwxrwxrwx permission files with /su and /sudo method
Date: Tue, 30 Jun 2009 14:21:53 +0200

Teemu Likonen <tlikonen <at> iki.fi> writes:

>> Thanks. Otherwise OK but I don't like the fact that it gives executable
>> bits (-rwxr-xr-x) by default. Normal behavior for new files is to drop
>> umask bits _and_ executable bits. Executable must be added explicitly.

When creating a new file, Tramp uses Emacs' default file modes. You can
check them with "M-: (default-file-modes)".

If you want to change them, you could apply for example
"M-: (set-default-file-modes #o0400)". The value is used then for all
newly created files, also for local ones.

> And when editing existing files it should obviously respect the bits
> that the file already has. Currently -- even with this fix -- tramp is
> adding "x" bits at some point because "backup by rename" moves old
> version out of the way and new is created with -rwxr-xr-x bits.

As I said already, I cannot reproduce it. However, there seems to be a
small annoyance in special cases. I've fixed this. Could you, please,
check, whether it is OK now for you?

Best regards, Michael.

Message #39 received at 3712 <at> emacsbugs.donarmstrong.com (full text, mbox):

From: Teemu Likonen <tlikonen <at> iki.fi>
To: Michael Albinus <michael.albinus <at> gmx.de>
Cc: 3712 <at> debbugs.gnu.org
Subject: Re: bug#3712: 23.1.50; SECURITY: Tramp creates -rwxrwxrwx permission files with /su and /sudo method
Date: Tue, 30 Jun 2009 16:57:26 +0300

On 2009-06-30 14:21 (+0200), Michael Albinus wrote:

> When creating a new file, Tramp uses Emacs' default file modes. You
> can check them with "M-: (default-file-modes)".
>
> If you want to change them, you could apply for example "M-:
> (set-default-file-modes #o0400)". The value is used then for all newly
> created files, also for local ones.

Hmm, I didn't know about those functions, thanks. And I compiled my
Emacs with your recent changes too.

I still don't like the default difference between creating a file as a
normal user or through /su: or /sudo:. Here's again an example starting
from command

    umask 0022; emacs -Q

When I create a file without Tramp (C-x C-f ~/test.txt RET) to my home
directory it gets bits 0644. When I create a file through Tramp to
/sudo::/root/test.txt it gets bits 0755 (i.e. with executable bits). In
both cases Emacs's default-file-modes is the same, the untouched default
which is #o755. In fact, all the settings are the same.

I'm not sure where this difference should be fixed but from user's point
of view the Tramp part brings the unexpected end result. It's unexpected
because no other programs create new executable files by default, even
when umask doesn't mask executable bits.

I appreciate your hint about set-default-file-modes, and I'll use it if
necessary, but in my opinion user shouldn't need to run

    (set-default-file-modes #o0644)

in her ~/.emacs just because she wants Tramp to behave similarly to her
umask=0022 settings. Instead, the similar behavior should be the
default.

>> And when editing existing files it should obviously respect the bits
>> that the file already has. Currently -- even with this fix -- tramp
>> is adding "x" bits at some point because "backup by rename" moves old
>> version out of the way and new is created with -rwxr-xr-x bits.
>
> As I said already, I cannot reproduce it. However, there seems to be a
> small annoyance in special cases. I've fixed this. Could you, please,
> check, whether it is OK now for you?

I could reproduce it before but it seems that not anymore with your
newest changes. If you want clear steps how to reproduce it I can
inspect the issue more closely.

Anyway, thanks for your work on Tramp and Emacs! :-)

Message #44 received at 3712 <at> emacsbugs.donarmstrong.com (full text, mbox):

From: Michael Albinus <michael.albinus <at> gmx.de>
To: Teemu Likonen <tlikonen <at> iki.fi>
Cc: 3712 <at> debbugs.gnu.org
Subject: Re: bug#3712: 23.1.50; SECURITY: Tramp creates -rwxrwxrwx permission files with /su and /sudo method
Date: Tue, 30 Jun 2009 17:34:27 +0200

Teemu Likonen <tlikonen <at> iki.fi> writes:

> I'm not sure where this difference should be fixed but from user's point
> of view the Tramp part brings the unexpected end result. It's unexpected
> because no other programs create new executable files by default, even
> when umask doesn't mask executable bits.

OK, you've convinced me. Execution bits are removed now for newly
created remote files.

>> As I said already, I cannot reproduce it. However, there seems to be a
>> small annoyance in special cases. I've fixed this. Could you, please,
>> check, whether it is OK now for you?
>
> I could reproduce it before but it seems that not anymore with your
> newest changes. If you want clear steps how to reproduce it I can
> inspect the issue more closely.

If it works also for you it is OK for me.

> Anyway, thanks for your work on Tramp and Emacs! :-)

Best regards, Michael.

Message #49 received at 3712-done <at> emacsbugs.donarmstrong.com (full text, mbox):

From: Teemu Likonen <tlikonen <at> iki.fi>
To: Michael Albinus <michael.albinus <at> gmx.de>
Cc: 3712-done <at> debbugs.gnu.org
Subject: Re: bug#3712: 23.1.50; SECURITY: Tramp creates -rwxrwxrwx permission files with /su and /sudo method
Date: Tue, 30 Jun 2009 19:36:32 +0300

On 2009-06-30 17:34 (+0200), Michael Albinus wrote:

> OK, you've convinced me. Execution bits are removed now for newly
> created remote files.

> If it works also for you it is OK for me.

It seems to work perfectly now. Huge thanks! I'm happy to close this
bug.

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.