Re: "here strings" and tmpfiles

[Jason A. Donenfeld]

I keep forgetting things. The other thing I wanted to bring up is that
I suspect bash's actual implementation of temporary files is
problematic and might have some of the classic /tmp and TOCTOU style
attacks. The current implementation is three-fold via ifdefs:

char *
sh_mktmpname (nameroot, flags)
    char *nameroot;
    int flags;
{
 char *filename, *tdir, *lroot;
 struct stat sb;
 int r, tdlen;
 static int seeded = 0;

 filename = (char *)xmalloc (PATH_MAX + 1);
 tdir = get_tmpdir (flags);
 tdlen = strlen (tdir);

 lroot = nameroot ? nameroot : DEFAULT_NAMEROOT;

#ifdef USE_MKTEMP
 sprintf (filename, "%s/%s.XXXXXX", tdir, lroot);
 if (mktemp (filename) == 0)
   {
     free (filename);
     filename = NULL;
   }
#else  /* !USE_MKTEMP */
 sh_seedrand ();
 while (1)
   {
     filenum = (filenum << 1) ^
               (unsigned long) time ((time_t *)0) ^
               (unsigned long) dollar_dollar_pid ^
               (unsigned long) ((flags & MT_USERANDOM) ? random () :
ntmpfiles++);
     sprintf (filename, "%s/%s-%lu", tdir, lroot, filenum);
     if (tmpnamelen > 0 && tmpnamelen < 32)
       filename[tdlen + 1 + tmpnamelen] = '\0';
#  ifdef HAVE_LSTAT
     r = lstat (filename, &sb);
#  else
     r = stat (filename, &sb);
#  endif
     if (r < 0 && errno == ENOENT)
       break;
   }
#endif /* !USE_MKTEMP */

 return filename;
}

The first one there uses mktemp(3), which is known to be racy and
insecure. The GNU man page has a pretty strong warning about it. Maybe
that's not used in GNU environments though?

The second one uses sh_seedrand(), which uses the bogus and
predictable random() libc stuff, or does nothing:

static void
sh_seedrand ()
{
#if HAVE_RANDOM
 int d;
 static int seeded = 0;
 if (seeded == 0)
   {
     struct timeval tv;

     gettimeofday (&tv, NULL);
     srandom (tv.tv_sec ^ tv.tv_usec ^ (getpid () << 16) ^ (uintptr_t)&d);
     seeded = 1;
   }
#endif
}

It then goes on to include a bunch of other things xored together and
whatnot, none of which are actually unpredictable.

Finally, in some cases where there isn't lstat, it uses plain stat
instead, which has problems. But even in the case where there's lstat,
it's calling it on the filename before opening it, resulting in
TOCTOU.

I haven't spent the 10 minutes reading onward at context and whatnot
to actually see what exactly you're doing; it's possible that you've
thought through all this and its particular use is fine somehow. But
from a cursory birdseye look, I wonder if there are three CVEs lurking
in here (mktemp, random stuff, toctou).

So just FYI.

But we could also avoid this entire tempfile creation discussion by
getting rid of the need for those functions in the first place. :)

Jason