Subject: mkstemp(), doh!

From: Michael Han <[email-masked]>
Date: Sat, 24 Feb 2001 12:38:16 -0800

So, after adding a prviate version of mkstemp(), I started looking at
the places that tempnam() and mktemp() are being used, and I have a
hard time seeing how mkstemp() could be substituted for what's going
on. In session.c, tempnam() is used, but it looks to me like it's used
for stashing the session state when fvwm closes. Naturally, the next
window manager to come along needs to know what this file is called.
Given the mkstemp() only returns a file descriptor, and, as far as I
can tell, doesn't *necessarily* modify the template to show what it
did, I think it's going to be tough to replace tempnam() with
mkstemp().

Similarly, FvwmCpp and FvwmM4 have the same routine for deailng with
the temporary config file. They call mktemp(), open the resulting temp
file, and then pass that file as an argument to the preprocessor via
popen(m4 > temp); That's again going to be tough to replace with
mkstemp... Maybe if it were popen(m4 |) we could attach stdout from
the popen to the fd we get from mkstemp(), but I'm out of my depth
doing that...

So, anyone got any hints? Can we rely on the side-effect of the
template being modified, so we can recheck *template after passing
through mkstemp() to see what filename it used? Or should I just
scratch the whole mkstemp() idea and back out my commit? Ideas?
-- 
[email-masked]                            http://www.mikehan.com/
coffee achiever                              San Francisco, California
To understand recursion you must first understand recursion.
--
Visit the official FVWM web page at <URL:http://www.fvwm.org/>.
To unsubscribe from the list, send "unsubscribe fvwm-workers" in the
body of a message to [email-masked].
To report problems, send mail to [email-masked]

---

From: Dan Espen <[email-masked]>
Date: Sat, 24 Feb 2001 21:13:30 -0500

Michael Han <[email-masked]> writes:
> I have a
> on.
...
> scratch the whole mkstemp() idea and back out my commit? Ideas?

This was the original problem:

FvwmCpp.o: In function `cpp_defs':
/home/sa/src/fvwm/modules/FvwmCpp/FvwmCpp.c:255: the use of `mktemp'
is dangerous, better use `mkstemp'

I'm guessing that the danger is this (Solaris man page):

     Between the time a file name is  created  and  the  file  is
     opened,  it  is  possible for some other process to create a
     file with the same name.

The Solaris man page continues:

     This can never happen if that other
     process  is using these functions or mktemp(3C) and the file
     names are  chosen  to  render  duplication  by  other  means
     unlikely.

The HP man page reads the same way.

--
Dan Espen
444 Hoes Lane  Room RRC 1C-214           E-mail: [email-masked]
Piscataway, NJ 08854                     Phone: (732) 699-5570
--
Visit the official FVWM web page at <URL:http://www.fvwm.org/>.
To unsubscribe from the list, send "unsubscribe fvwm-workers" in the
body of a message to [email-masked].
To report problems, send mail to [email-masked]

---

From: Michael Han <[email-masked]>
Date: Sun, 25 Feb 2001 11:34:24 -0800

On Sat, Feb 24, 2001 at 09:13:30PM -0500, Dan Espen wrote:
> [snip]

The bigger danger, especially for something like FvwmCpp and FvwmM4,
which are producing "executable" code that will be run under a user
context, is in a race in which a local user subverts the tempfile's
contents to trojan something like "Exec rm -rf /". Because the
preprocessor modules destructively open the tempfile sort of like
this: popen("m4 -foo > tmp_name", "w"), if the file is created before
popen() but after the post-mktemp() tests, it can be made with unsafe
modes like 664, allowing arbitrary data to be inserted. FvwmCpp or
FvwmM4 would then blindly feed this data to the FVWM core. The user's
umask may also allow this to happen -- there are at least a few
systems I've worked on where the default umask was 2, not 22.

The more I think about this, the more I think that the right thing to
do is to use mkstemp() and figure out a way to pass around a
read/write fd from mkstemp, rather than actual file names.

I don't really understand the stuff in session.c yet, so I can't
really offer much comment on the use of tempnam() there.
-- 
[email-masked]                            http://www.mikehan.com/
coffee achiever                              San Francisco, California
Avoid cliches like the plague
--
Visit the official FVWM web page at <URL:http://www.fvwm.org/>.
To unsubscribe from the list, send "unsubscribe fvwm-workers" in the
body of a message to [email-masked].
To report problems, send mail to [email-masked]

---

From: Dan Espen <[email-masked]>
Date: Sun, 25 Feb 2001 19:10:05 -0500

...
> read/write fd from mkstemp, rather than actual file names.

OK, I was just trying to give you an easy way out.

I really don't think there is much of an opening there.
The open, write, close, open, read cycle is going to complete
pretty quickly.

---

From: Dominik Vogt <[email-masked]>
Date: Mon, 26 Feb 2001 13:50:04 +0100

> did, I think it's going to be tough to replace tempnam() with

On Linux the man page states that the template *will* be modified,
not that it *may* be modified:

       The  mkstemp()  function generates a unique temporary file
       name from template.  The last six characters  of  template
       must  be  XXXXXX and these are replaced with a string that
                            ^^^^^^^^^^^^^^^^^^
       makes the filename unique.

Also, if it were differently you could never delete this file
again.

Anyway, configure already has to check if mkstemp is supported on
the system, so it should not be hard too check if mkstemp also
modified the template:

  #include <stdlib.h>
  #include <string.h>
  int main(void)
  {
    char template[128];
    char template_orig[128]
    int fd;

    sprintf(template, "configure-mkstemp-test.XXXXXX");
    strcpy(template_orig, template);
    fd = mkstemp(template);
    if (fd == -1)
    {
      /* could not create temp file */
      return 1;
    }
    if (strcmp(template, template_orig) == 0)
    {
      /* mkstemp broken */
      return 2;
    }
    if (fclose(fd) == EOF)
    {
      /* doh! */
      return 3;
    }
    if (unlink(template))
    {
      /* doh! */
      return 4;
    }
    /* mkstemp works properly */
    return 0;
  }

If that program can not be compiled, linked and run or if it
returns with an error, we'd use our drop in replacement.

Bye

Dominik ^_^  ^_^

--
Dominik Vogt, [email-masked]
Reply-To: [email-masked]
--
Visit the official FVWM web page at <URL:http://www.fvwm.org/>.
To unsubscribe from the list, send "unsubscribe fvwm-workers" in the
body of a message to [email-masked].
To report problems, send mail to [email-masked]