bug#17187: 24.3.50.1 open-dribble-file stores pw

bug#17187: 24.3.50.1 open-dribble-file stores pw

[Andreas Röhler]

Emacs -Q from 2014-02-19

Passwort gets stored in plain text

bug#17187: 24.3.50.1 open-dribble-file stores pw

[Glenn Morris]

As suggested a decade ago,

http://lists.gnu.org/archive/html/emacs-pretest-bug/2003-10/msg00229.html

the dribble file should be created with file permission bits = 600.

bug#17187: 24.3.50.1 open-dribble-file stores pw

[Andreas Röhler]

Am 04.04.2014 23:42, schrieb Glenn Morris:
>
> As suggested a decade ago,
>
> http://lists.gnu.org/archive/html/emacs-pretest-bug/2003-10/msg00229.html
>
> the dribble file should be created with file permission bits = 600.

So why Emacs doesn't set permissions accordingly?

bug#17187: 24.3.50.1 open-dribble-file stores pw

[Andreas Röhler]

Am 04.04.2014 23:42, schrieb Glenn Morris:
>
> As suggested a decade ago,
>
> http://lists.gnu.org/archive/html/emacs-pretest-bug/2003-10/msg00229.html
>
> the dribble file should be created with file permission bits = 600.
>

BTW IMHO it's a serious security-hole, should be flagged accordingly.
There will be numerous users with these kind of stuff during session.

bug#17187: 24.3.50.1 open-dribble-file stores pw

[Stefan Monnier]

severity 17187 important
thanks

> As suggested a decade ago,
> http://lists.gnu.org/archive/html/emacs-pretest-bug/2003-10/msg00229.html
> the dribble file should be created with file permission bits = 600.

Very much agreed.


        Stefan

bug#17187: 24.3.50.1 open-dribble-file stores pw

[Andreas Röhler]

Am 05.04.2014 17:50, schrieb Stefan Monnier:
> severity 17187 important
> thanks
>
>> As suggested a decade ago,
>> http://lists.gnu.org/archive/html/emacs-pretest-bug/2003-10/msg00229.html
>> the dribble file should be created with file permission bits = 600.
>
> Very much agreed.
>
>
>          Stefan
>

Will that solve the matter already? IMO a pw should never be stored as plain-text.
File-permissions are not considered save in that context.

Should be a way to replace the chars by "*" for example before writing it.

Andreas

bug#17187: 24.3.50.1 open-dribble-file stores pw

[Andreas Schwab]

Andreas Röhler <andreas.roehler@easy-emacs.de> writes:

> Will that solve the matter already? IMO a pw should never be stored as plain-text.

The dribble file does not know what a password is.

Andreas.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

bug#17187: 24.3.50.1 open-dribble-file stores pw

[Glenn Morris]

Stefan Monnier wrote:

>> As suggested a decade ago,
>> http://lists.gnu.org/archive/html/emacs-pretest-bug/2003-10/msg00229.html
>> the dribble file should be created with file permission bits = 600.
>
> Very much agreed.

PS maybe it should also abort with an error if the file already exists
(and is a symlink or is not owned by the current user?).

bug#17187: 24.3.50.1 open-dribble-file stores pw

[Andreas Röhler]

Am 05.04.2014 18:55, schrieb Andreas Schwab:
> Andreas Röhler <andreas.roehler@easy-emacs.de> writes:
>
>> Will that solve the matter already? IMO a pw should never be stored as plain-text.
>
> The dribble file does not know what a password is.
>
> Andreas.
>

As Emacs shell sent as prompt for pw, at least Emacs knows.
All remains to do is to ship that info.

bug#17187: 24.3.50.1 open-dribble-file stores pw

[Andreas Schwab]

Andreas Röhler <andreas.roehler@easy-emacs.de> writes:

> Am 05.04.2014 18:55, schrieb Andreas Schwab:
>> Andreas Röhler <andreas.roehler@easy-emacs.de> writes:
>>
>>> Will that solve the matter already? IMO a pw should never be stored as plain-text.
>>
>> The dribble file does not know what a password is.
>>
>> Andreas.
>>
>
> As Emacs shell sent as prompt for pw, at least Emacs knows.

Not at this level.

Andreas.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

bug#17187: 24.3.50.1 open-dribble-file stores pw

[Stefan Monnier]

>>> As suggested a decade ago,
>>> http://lists.gnu.org/archive/html/emacs-pretest-bug/2003-10/msg00229.html
>>> the dribble file should be created with file permission bits = 600.
>> Very much agreed.
> PS maybe it should also abort with an error if the file already exists
> (and is a symlink or is not owned by the current user?).

You mean it should be created with EXCL?
Maybe.  Then again, AFAIK this is only used for debugging purposes, so
I'm not sure it's that important and you could assume that the user will
normally specify a file in a directory she owns, where the attacker
shouldn't be able to place a surreptitious symlink.


        Stefan

bug#17187: 24.3.50.1 open-dribble-file stores pw

[Glenn Morris]

Lightly tested:

*** src/keyboard.c	2014-04-05 18:33:55 +0000
--- src/keyboard.c	2014-04-05 22:59:00 +0000
***************
*** 20,25 ****
--- 20,26 ----
  #include <config.h>
  
  #include "sysstdio.h"
+ #include <sys/stat.h>
  
  #include "lisp.h"
  #include "termchar.h"
***************
*** 10085,10092 ****
      }
    if (!NILP (file))
      {
        file = Fexpand_file_name (file, Qnil);
!       dribble = emacs_fopen (SSDATA (file), "w");
        if (dribble == 0)
  	report_file_error ("Opening dribble", file);
      }
--- 10086,10100 ----
      }
    if (!NILP (file))
      {
+       int fd;
        file = Fexpand_file_name (file, Qnil);
!       if (! NILP (Ffile_exists_p (file)))
!         {
!           if (chmod (SSDATA (file), 0600) < 0)
!             report_file_error ("Doing chmod", file);
!         }
!       fd = emacs_open (SSDATA (file), O_WRONLY | O_CREAT | O_TRUNC, 0600);
!       dribble = fd < 0 ? 0 : fdopen (fd, "w");
        if (dribble == 0)
  	report_file_error ("Opening dribble", file);
      }

bug#17187: 24.3.50.1 open-dribble-file stores pw

[Daniel Colascione]

[-- Attachment #1: Type: text/plain, Size: 934 bytes --]

On 04/05/2014 04:01 PM, Glenn Morris wrote:
> ***************
> *** 10085,10092 ****
>       }
>     if (!NILP (file))
>       {
>         file = Fexpand_file_name (file, Qnil);
> !       dribble = emacs_fopen (SSDATA (file), "w");
>         if (dribble == 0)
>   	report_file_error ("Opening dribble", file);
>       }
> --- 10086,10100 ----
>       }
>     if (!NILP (file))
>       {
> +       int fd;
>         file = Fexpand_file_name (file, Qnil);
> !       if (! NILP (Ffile_exists_p (file)))
> !         {
> !           if (chmod (SSDATA (file), 0600) < 0)
> !             report_file_error ("Doing chmod", file);
> !         }
> !       fd = emacs_open (SSDATA (file), O_WRONLY | O_CREAT | O_TRUNC, 0600);
> !       dribble = fd < 0 ? 0 : fdopen (fd, "w");
>         if (dribble == 0)

That's racy. What about using fchmod and falling back to post-open chmod
for systems that don't have fchmod?


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 901 bytes --]

bug#17187: 24.3.50.1 open-dribble-file stores pw

[Glenn Morris]

Daniel Colascione wrote:

> That's racy. What about using fchmod and falling back to post-open chmod
> for systems that don't have fchmod?

I'm no C coder, please feel free to improve it.
But IIUC it's been argued that we don't need to guard against malicious
intent here, only user oversight.

bug#17187: 24.3.50.1 open-dribble-file stores pw

[Glenn Morris]

Version: 24.4

File now created private.