Re: another /tmp race: `perl -e' opens temp file not safely

From: Theo de Raadt (deraadtat_private)
Date: Sat Mar 07 1998 - 18:06:25 PST

  • Next message: stanislav shalunov: "Re: another /tmp race: `perl -e' opens temp file not safely"

    This PERL problem was fixed by me in OpenBSD in early _1997_.  The
    patch I made to perl 5.003 was commited with the following log entry:
    
      revision 1.2
      date: 1997/01/23 04:31:36;  author: deraadt;  state: Exp;  lines: +9 -5
      perl mktemp race; fix mailed to larry
    
    Note that I sent Larry mail about the problem, but this did not result
    in a fix shipping in 5.004_04.  Bad Larry!  What other perl security
    problems have not gotten fixed?
    
    Moreover, Stanislav's patch retained the use of of mktemp(), which
    means his fixed perl can still be denial-of-serviced (because another
    process can guess the name and create a file there and then the perl
    -e job aborts).  The correct fix is to use mkstemp(), which is
    designed to be (much more) impervious to these kinds of races.
    
    Two patches are appended below.
    
    The first patch is my original fix for 5.003:
    
    Index: perl.c
    ===================================================================
    RCS file: /cvs/src/gnu/usr.bin/perl/perl.c,v
    retrieving revision 1.1
    retrieving revision 1.2
    diff -u -r1.1 -r1.2
    --- perl.c      1996/08/19 10:11:44     1.1
    +++ perl.c      1997/01/23 04:31:36     1.2
    @@ -337,13 +337,17 @@
                if (euid != uid || egid != gid)
                    croak("No -e allowed in setuid scripts");
                if (!e_fp) {
    +               int fd;
    +
                    e_tmpname = savepv(TMPPATH);
    -               (void)mktemp(e_tmpname);
    -               if (!*e_tmpname)
    -                   croak("Can't mktemp()");
    -               e_fp = fopen(e_tmpname,"w");
    -               if (!e_fp)
    +               fd = mkstemp(e_tmpname);
    +               if (fd == -1)
    +                   croak("Can't mkstemp()");
    +               e_fp = fdopen(fd,"w");
    +               if (!e_fp) {
    +                   close(fd);
                        croak("Cannot open temporary file");
    +               }
                }
                if (argv[1]) {
                    fputs(argv[1],e_fp);
    
    
    This second patch is the one made by Todd Miller when he merged my fix
    forward into perl 5.004_04: This is a nicer fix.  I think PerlIO_fdopen()
    didn't exist in 5.003, but whatever.
    
    --- perl.c.orig Tue Oct 14 12:09:18 1997
    +++ perl.c      Sun Nov 30 00:48:55 1997
    @@ -588,13 +588,17 @@
                if (euid != uid || egid != gid)
                    croak("No -e allowed in setuid scripts");
                if (!e_fp) {
    +               int fd;
    +
                    e_tmpname = savepv(TMPPATH);
    -               (void)mktemp(e_tmpname);
    -               if (!*e_tmpname)
    -                   croak("Can't mktemp()");
    -               e_fp = PerlIO_open(e_tmpname,"w");
    -               if (!e_fp)
    +               fd = mkstemp(e_tmpname);
    +               if (fd == -1)
    +                   croak("Can't mkstemp()");
    +               e_fp = PerlIO_fdopen(fd,"w");
    +               if (!e_fp) {
    +                   (void)close(fd);
                        croak("Cannot open temporary file");
    +               }
                }
                if (*++s)
                    PerlIO_puts(e_fp,s);
    
    
    I guess I get to say it again: This security problem was fixed in
    OpenBSD more than a year ago.
    



    This archive was generated by hypermail 2b30 : Fri Apr 13 2001 - 13:44:27 PDT