temp file hole?

temp file hole?

[Stefan Monnier]

I'm afraid that the recent change to tramp-make-temp-file to use
makw-temp-name instead of make-temp-file introduced a security hole (the
very hole plugged by the introduction of make-temp-file in the first place).

I.e. if you want to keep using make-temp-name, please justify with comments
why your code is not vulnerable to the usual temp-file race condition
which goes something like:
1- Tramp decides to use /tmp/foo1234 as temp file.
2- some attacker creates a symlink from /tmp/foo1234 to some interesting place.
3- Tramp writes to /tmp/foo1234 without realizing that it's actually writing
   to the interesting place through that symlink.

The make-temp-name docstring also says:

   There is a race condition between calling `make-temp-name' and creating the
   file which opens all kinds of security holes.  For that reason, you should
   probably use `make-temp-file' instead, except in three circumstances:
   
   * If you are creating the file in the user's home directory.
   * If you are creating a directory rather than an ordinary file.
   * If you are taking special precautions as `make-temp-file' does.


-- Stefan

Re: temp file hole?

[Michael Albinus]

Stefan Monnier <monnier@iro.umontreal.ca> writes:

Hi Stefan,

> I'm afraid that the recent change to tramp-make-temp-file to use
> makw-temp-name instead of make-temp-file introduced a security hole (the
> very hole plugged by the introduction of make-temp-file in the first place).

I confess that my Changelog entry is a little bit sloppy. And you are
right, there could be a security hole.

The other reason why I have switched from make-temp-file to
make-temp-name is that make-temp-file creates a file without the
possibility to declare a file name extension. In Tramp, it is
sometimes useful to have the same file name extension in both the
temporary file and the original file the temporary file is used
for. By this, some actions like deciding major mode etc works
automatically.

A solution could be that tramp-make-temp-file takes over part of the
implementation of make-temp-file, i.e. applies make-temp-name, adds
the desired extension to this file name, and creates immediately the
temporary file via a loop like in make-temp-file.

A similar approach should be applied to tramp-make-tramp-temp-file,
which suffers from this security hole since ever.

I'll commit a patch tonight (or tomorrow, depends on my spare time).

> -- Stefan

Thanks, and best regards, Michael.

Re: temp file hole?

[Johan Bockgård]

Michael Albinus <michael.albinus@gmx.de> writes:

> The other reason why I have switched from make-temp-file to
> make-temp-name is that make-temp-file creates a file without the
> possibility to declare a file name extension.

FWIW, the possibility exists in Emacs 22

   (make-temp-file PREFIX &optional DIR-FLAG SUFFIX)

-- 
Johan Bockgård

Re: temp file hole?

[Michael Albinus]

I wrote:

> The other reason why I have switched from make-temp-file to
> make-temp-name is that make-temp-file creates a file without the
> possibility to declare a file name extension.

This is true for Emacs 21. For Emacs 22/23, I can use make-temp-file
as it is, of course.

Best regards, Michael.

Re: temp file hole?

[Ulrich Mueller]

>>>>> On Thu, 04 Oct 2007, Michael Albinus wrote:

> And you are right, there could be a security hole.

> [...]

> A similar approach should be applied to tramp-make-tramp-temp-file,
> which suffers from this security hole since ever.

> I'll commit a patch tonight (or tomorrow, depends on my spare time).

I noticed that a patch for tramp-make-temp-file was committed, but
tramp-make-tramp-temp-file is unchanged. Does this mean that the
latter is not suffering from the problem?

Ulrich

Re: temp file hole?

[Michael Albinus]

Ulrich Mueller <ulm@gentoo.org> writes:

>> A similar approach should be applied to tramp-make-tramp-temp-file,
>> which suffers from this security hole since ever.
>
>> I'll commit a patch tonight (or tomorrow, depends on my spare time).
>
> I noticed that a patch for tramp-make-temp-file was committed, but
> tramp-make-tramp-temp-file is unchanged. Does this mean that the
> latter is not suffering from the problem?

It means that I'm still working on it. My patch runs OK with GNU Emacs
22/23 already, but I'll perform further compatibility tests with GNU
Emacs 21 and XEmacs. Will be committed when I'm ready.

> Ulrich

Best regards, Michael.

Re: temp file hole?

[Stefan Monnier]

>> I'm afraid that the recent change to tramp-make-temp-file to use
>> makw-temp-name instead of make-temp-file introduced a security hole (the
>> very hole plugged by the introduction of make-temp-file in the first place).

> I confess that my Changelog entry is a little bit sloppy. And you are
> right, there could be a security hole.

Indeed, I also looked at the code, and I'm pretty sure there's
a security hole.

> The other reason why I have switched from make-temp-file to
> make-temp-name is that make-temp-file creates a file without the
> possibility to declare a file name extension. In Tramp, it is

Then please use make-temp-file when the SUFFIX arg is available, so at least
the security hole is plugged in Emacs-22.


        Stefan

Re: temp file hole?

[Michael Albinus]

Stefan Monnier <monnier@iro.umontreal.ca> writes:

>> The other reason why I have switched from make-temp-file to
>> make-temp-name is that make-temp-file creates a file without the
>> possibility to declare a file name extension. In Tramp, it is
>
> Then please use make-temp-file when the SUFFIX arg is available, so at least
> the security hole is plugged in Emacs-22.

That's what I've done meanwhile, see tramp-compat.el.

>         Stefan

Best regards, Michael.

Re: temp file hole?

[Michael Albinus]

Ulrich Mueller <ulm@gentoo.org> writes:

> I noticed that a patch for tramp-make-temp-file was committed, but
> tramp-make-tramp-temp-file is unchanged. Does this mean that the
> latter is not suffering from the problem?

I've committed the patch for tramp-make-tramp-temp-file.

> Ulrich

Best regards, Michael.

Re: temp file hole?

[Ulrich Mueller]

This issue has a CVE number now: CVE-2007-5377

<http://nvd.nist.gov/nvd.cfm?cvename=CVE-2007-5377>
<http://bugs.gentoo.org/show_bug.cgi?id=194713>

Ulrich

Re: temp file hole?

[Michael Albinus]

Ulrich Mueller <ulm@gentoo.org> writes:

> This issue has a CVE number now: CVE-2007-5377
>
> <http://nvd.nist.gov/nvd.cfm?cvename=CVE-2007-5377>
> <http://bugs.gentoo.org/show_bug.cgi?id=194713>

Thanks. Tramp 2.1.11 has been released recently, which contains the fix.

> Ulrich

Best regards, Michael.