perldoc security bug (race condition)

[p5pRT]

Migrated from rt.perl.org#2095 (status was 'resolved')

Searchable as RT2095$

[p5pRT]

From @jmaslak

This is a bug report for perl from jmaslak@​mindspring.com\, generated with the help of perlbug 1.26 running under perl 5.00503.

---

perldoc uses /tmp/perldoc1.[PID] for it's temporary files. I was able to confirm that I am able to overwrite files by creating symbolic links of the form /tmp/perldoc1.[PID] .

This is due to perldoc not using a secure method to generate a temporary file\, such as mkstemp().

An example exploit would be to send root a mail saying\, "I am having trouble getting perldoc to work correctly. Use this command​: "perldoc /home/mydir/sample.pm" to see what I mean.

/home/mydir/sample.pm would output simply two plusses. As you can see\, this could be very dangerous.

(perlbug also seems to have a similar problem after a quick but less thorough examination)

-- Joel Maslak

---

---

This perlbug was built using Perl 5.00503 - Wed Oct 20 00​:47​:06 MEST 1999 It is being executed now by Perl 5.00503 - Tue Apr 6 23​:33​:05 EDT 1999.

Site configuration information for perl 5.00503​:

Configured by root at Tue Apr 6 23​:33​:05 EDT 1999.

Summary of my perl5 (5.0 patchlevel 5 subversion 3) configuration​:   Platform​:   osname=linux\, osvers=2.2.1-ac1\, archname=i386-linux   uname='linux porky.devel.redhat.com 2.2.1-ac1 #1 smp mon feb 1 17​:44​:44 est 1999 i686 unknown '   hint=recommended\, useposix=true\, d_sigaction=define   usethreads=undef useperlio=undef d_sfio=undef   Compiler​:   cc='cc'\, optimize='-O2'\, gccversion=egcs-2.91.66 19990314/Linux (egcs-1.1.2 release)   cppflags='-Dbool=char -DHAS_BOOL -I/usr/local/include'   ccflags ='-Dbool=char -DHAS_BOOL -I/usr/local/include'   stdchar='char'\, d_stdstdio=undef\, usevfork=false   intsize=4\, longsize=4\, ptrsize=4\, doublesize=8   d_longlong=define\, longlongsize=8\, d_longdbl=define\, longdblsize=12   alignbytes=4\, usemymalloc=n\, prototype=define   Linker and Libraries​:   ld='cc'\, ldflags =' -L/usr/local/lib'   libpth=/usr/local/lib /lib /usr/lib   libs=-lnsl -lndbm -lgdbm -ldb -ldl -lm -lc -lposix -lcrypt   libc=\, so=so\, useshrplib=false\, libperl=libperl.a   Dynamic Linking​:   dlsrc=dl_dlopen.xs\, dlext=so\, d_dlsymun=undef\, ccdlflags='-rdynamic'   cccdlflags='-fpic'\, lddlflags='-shared -L/usr/local/lib'

Locally applied patches​:  

---

@​INC for perl 5.00503​:   /usr/lib/perl5/5.00503/i386-linux   /usr/lib/perl5/5.00503   /usr/lib/perl5/site_perl/5.005/i386-linux   /usr/lib/perl5/site_perl/5.005   .

---

Environment for perl 5.00503​:   HOME=/home/jmaslak   LANG (unset)   LANGUAGE (unset)   LD_LIBRARY_PATH (unset)   LOGDIR (unset)   PATH=/usr/local/bin​:/bin​:/usr/bin​:/usr/local/bin​:/bin​:/usr/bin​:/usr/X11R6/bin​:/home/jmaslak/bin​:/home/jmaslak/bin​:/usr/X11R6/bin​:/home/jmaslak/bin   PERL_BADLANG (unset)   SHELL=/bin/bash

[p5pRT]

From [Unknown Contact. See original ticket]

This is what you get for not using the real man program when you're supposed to.

--tom

[p5pRT]

From @chipdude

According to Joel Maslak​:

This is due to perldoc not using a secure method to generate a temporary file\, such as mkstemp().

Does this look like a good feature to build into Perl? Temporary files are such a common need\, and they're so easily done wrong. -- Chip Salzenberg - a.k.a. - \chip@​valinux\.com "Come back to AT&T!" "We're your friends and family now!" //MST3K

[p5pRT]

From @jhi

Chip Salzenberg writes​:

According to Joel Maslak​:

This is due to perldoc not using a secure method to generate a temporary file\, such as mkstemp().

Does this look like a good feature to build into Perl? Temporary files are such a common need\, and they're so easily done wrong. --

We actually used to have mkstemp() probing in Configure for a short fleeting moment just before we eradicated the use of temp files by -e.

Yes\, I think having mkstemp() (not necessarily directly as a Perl function call\, but underneath somewhere\, File​::Temp->new()\, for example)\, would be a great thing. If there's no mkstemp()\, we can still recode the mkstemp() logic in Perl (sysopen(... O_EXCL|O_CREAT...)).

Something akin for temp directories would be cool\, too.

Chip Salzenberg - a.k.a. - \chip@​valinux\.com "Come back to AT&T!" "We're your friends and family now!" //MST3K

-- $jhi++; # http​://www.iki.fi/jhi/   # There is this special biologist word we use for 'stable'.   # It is 'dead'. -- Jack Cohen

[p5pRT]

From @chipdude

According to Jarkko Hietaniemi​:

Yes\, I think having mkstemp() (not necessarily directly as a Perl function call\, but underneath somewhere\, File​::Temp->new()\, for example)\, would be a great thing.

I was thinking more along the lines of a one-parameter open​:   $name = open(my $fh); (For support of old code\, the parameter must not be a bareword.)

Something akin for temp directories would be cool\, too.

  $name = mkdir(0700) ? -- Chip Salzenberg - a.k.a. - \chip@​valinux\.com "Come back to AT&T!" "We're your friends and family now!" //MST3K

[p5pRT]

From @jhi

Chip Salzenberg writes​:

According to Jarkko Hietaniemi​:

Yes\, I think having mkstemp() (not necessarily directly as a Perl function call\, but underneath somewhere\, File​::Temp->new()\, for example)\, would be a great thing.

I was thinking more along the lines of a one-parameter open​: $name = open(my $fh);

Nice.

Would the temp file be unlinked automagically at scope exit?

(For support of old code\, the parameter must not be a bareword.)

Something akin for temp directories would be cool\, too.

$name = mkdir\(0700\)

And bare mkdir() defaulting to mkdir(0700)?

Would the directory by automagically recursively destructed at scope exit?

? -- Chip Salzenberg - a.k.a. - \chip@​valinux\.com "Come back to AT&T!" "We're your friends and family now!" //MST3K

-- $jhi++; # http​://www.iki.fi/jhi/   # There is this special biologist word we use for 'stable'.   # It is 'dead'. -- Jack Cohen

[p5pRT]

From @jhi

Jarkko Hietaniemi writes​:

Chip Salzenberg writes​:

According to Jarkko Hietaniemi​:

Yes\, I think having mkstemp() (not necessarily directly as a Perl function call\, but underneath somewhere\, File​::Temp->new()\, for example)\, would be a great thing.

I was thinking more along the lines of a one-parameter open​: $name = open(my $fh);

Nice.

Would the temp file be unlinked automagically at scope exit?

Oh\, yes of course it would\, thanks to the autovivification of filehandles...

-- $jhi++; # http​://www.iki.fi/jhi/   # There is this special biologist word we use for 'stable'.   # It is 'dead'. -- Jack Cohen

[p5pRT]

From @jhi

Jarkko Hietaniemi writes​:

Jarkko Hietaniemi writes​:

Chip Salzenberg writes​:

According to Jarkko Hietaniemi​:

Yes\, I think having mkstemp() (not necessarily directly as a Perl function call\, but underneath somewhere\, File​::Temp->new()\, for example)\, would be a great thing.

I was thinking more along the lines of a one-parameter open​: $name = open(my $fh);

Nice.

Would the temp file be unlinked automagically at scope exit?

Oh\, yes of course it would\, thanks to the autovivification of filehandles...

Rats. More coffee. *Much* more coffee. I was thinking of auto-close()\, auto-unlink() is _slightly_ different issue :-)

-- $jhi++; # http​://www.iki.fi/jhi/   # There is this special biologist word we use for 'stable'.   # It is 'dead'. -- Jack Cohen

[p5pRT]

From [Unknown Contact. See original ticket]

Jarkko Hietaniemi (lists.p5p)​:

I was thinking more along the lines of a one-parameter open​: $name = open(my $fh);

Nice.

Would the temp file be unlinked automagically at scope exit?

On co-operating operating systems\, you could do the old sleight-of-hand​: mkstemp\, create the file\, grab the filehandle and unlink the inode from the directory immediately\, dissociating it from the filesystem\, leaving the file open. I don't know what co-operates these days\, though.

-- I was sad because I had no shoes\, until I met a man who had no feet. So I said\, "Got any shoes you're not using?" -- Steven Wright

[p5pRT]

From @chipdude

According to Jarkko Hietaniemi​:

Chip Salzenberg writes​:

I was thinking more along the lines of a one-parameter open​: $name = open(my $fh); Nice. Would the temp file be unlinked automagically at scope exit?

I'd define it in terms of file closure​: "The file is automatically removed when the given filehandle is closed."

Under Unix\, of course\, the file would continue to exist as long as _any_ handle holds the given file open\, so it would be OK to immediately follow the open with an unlink. Unfortunately\, some systems frown on deleting open files\, so we can't count on that.

(For support of old code\, the parameter must not be a bareword.)

Something akin for temp directories would be cool\, too. $name = mkdir(0700) And bare mkdir() defaulting to mkdir(0700)?

Um\, I'm wondering whether automatic temp mkdir is a good idea after all ...

Would the directory by automagically recursively destructed at scope exit?

... OK\, I'm done wondering​: I don't like automatic temp mkdir. Any program doing anything heavy-duty enough to require a directory of its own can afford to load a module containing a robust tempname facility. Or of course we could put a tempname facility into the core\, in which case we don't need to mess with open(). -- Chip Salzenberg - a.k.a. - \chip@​valinux\.com "Come back to AT&T!" "We're your friends and family now!" //MST3K

[p5pRT]

From @jhi

Chip Salzenberg writes​:

I'd define it in terms of file closure​: "The file is automatically removed when the given filehandle is closed."

Under Unix\, of course\, the file would continue to exist as long as _any_ handle holds the given file open\, so it would be OK to immediately follow the open with an unlink. Unfortunately\, some systems frown on deleting open files\, so we can't count on that.

How about someone wanting to create temp files to somewhere else than /tmp/...XXXXXX? open(my $fh\, "/some/where/else/directory")?

(For support of old code\, the parameter must not be a bareword.)

Something akin for temp directories would be cool\, too. $name = mkdir(0700) And bare mkdir() defaulting to mkdir(0700)?

Um\, I'm wondering whether automatic temp mkdir is a good idea after all ...

Would the directory by automagically recursively destructed at scope exit?

... OK\, I'm done wondering​: I don't like automatic temp mkdir. Any program doing anything heavy-duty enough to require a directory of its own can afford to load a module containing a robust tempname facility.

Yeah\, seems like too much magic. File​::Temp​::mkdir\, or somesuch.

Or of course we could put a tempname facility into the core\, in which case we don't need to mess with open().

-- $jhi++; # http​://www.iki.fi/jhi/   # There is this special biologist word we use for 'stable'.   # It is 'dead'. -- Jack Cohen

[p5pRT]

From [Unknown Contact. See original ticket]

Chip Salzenberg \chip@​valinux\.com writes​:

I was thinking more along the lines of a one-parameter open​: $name = open(my $fh); (For support of old code\, the parameter must not be a bareword.)

Current code too; I use open(FOO) all the time. Did I miss a memo? :)

-- Russ Allbery (rra@​stanford.edu) \<URL​:http​://www.eyrie.org/~eagle/>

[p5pRT]

From [Unknown Contact. See original ticket]

Chip Salzenberg \chip@&#8203;valinux\.com writes​:

According to Joel Maslak​:

This is due to perldoc not using a secure method to generate a temporary file\, such as mkstemp().

Does this look like a good feature to build into Perl? Temporary files are such a common need\, and they're so easily done wrong.

In this particular case\, do we need a temporary file at all? What was the reason for using a temporary file rather than just a pipeline through the translator into the pager? Broken pipe errors from early exits from the pager?

-- Russ Allbery (rra@​stanford.edu) \<URL​:http​://www.eyrie.org/~eagle/>

[p5pRT]

From @chipdude

According to Jarkko Hietaniemi​:

How about someone wanting to create temp files to somewhere else than /tmp/...XXXXXX? open(my $fh\, "/some/where/else/directory")?

Mmm\, IMO\, that's over the magic line of too much magic. Perhaps there should be something in the core library\, if not the core language\, to create a new unique filename in the given directory. And we might want to make it always fail if the directory is 777.

  use File​::Unique qw/ unique_name unique_create /;

  my $t1 = unique_name; # /tmp   my $t2 = unique_name "$ENV{HOME}/tmp";

  mkdir($t1\, 0700); # mkdir is safe to use directly   my $f2 = unique_create($t2); # uses O_EXCL\, O_NOFOLLOW

  my $f3 = unique_create; # create in /tmp; auto-delete on close

Hm.
-- Chip Salzenberg - a.k.a. - \chip@&#8203;valinux\.com "Come back to AT&T!" "We're your friends and family now!" //MST3K

[p5pRT]

From [Unknown Contact. See original ticket]

Current code too; I use open(FOO) all the time. Did I miss a memo? :)

No\, that's perfectly supported. The only issue is that it's a package variable\, not the lexical\, which is consulted. Then again\, since the handle is also a package thingie\, this is reasonable.

--tom

[p5pRT]

From [Unknown Contact. See original ticket]

Tom Christiansen \tchrist@&#8203;chthon\.perl\.com writes​:

Current code too; I use open(FOO) all the time. Did I miss a memo? :)

No\, that's perfectly supported. The only issue is that it's a package variable\, not the lexical\, which is consulted. Then again\, since the handle is also a package thingie\, this is reasonable.

I pretty much only use it in conjunction with script globals. Usually when doing something like this​:

| # Command to generate the list of people with access to the NOTGS list. | $NOTGSGROUP = 'pts membership group-itss-crc​:passwdchange |';

[...]

| # Get NOTGS view information. | open NOTGSGROUP or die "Can't open $NOTGSGROUP​: $!\n"; | while (\) { | next if /Members of/; | s/\s+//g; | $perms{$_}{'notgs-view'} = 'Y'; | } | close NOTGSGROUP;

It somehow seems more self-documenting or cleaner or something than the two-argument version. That particular script has a whole set of globals at the top specifying information sources\, some of which are files and some of which are commands\, and then the script just opens barewords corresponding to each global and lets magic open take care of all the rest of the details.

-- Russ Allbery (rra@​stanford.edu) \<URL​:http​://www.eyrie.org/~eagle/>

[p5pRT]

From [Unknown Contact. See original ticket]

After quick checking\, I mis-identified this as a race condition. It is not -- the file's existence is never checked before it is created by perldoc.

As a fix that is SYSTEM DEPENDENT\, use of sysopen with a mode of O_CREAT|O_EXCL will work well\, although only on Unix and only on local filesystems. That's why I didn't send a patch. I'm not sure what the best way of fixing this is (especially in a way that doesn't break all the non-Unix systems).

I like the idea of a module to do this\, however. Perhaps it could use the "best solution" it could find on the host architecture... (some systems aren't going to have good methods of creating temporary files)

-- Joel

[p5pRT]

From [Unknown Contact. See original ticket]

Joel Maslak \jmaslak@&#8203;mindspring\.com writes​:

As a fix that is SYSTEM DEPENDENT\, use of sysopen with a mode of O_CREAT|O_EXCL will work well\, although only on Unix and only on local filesystems.

That seems sufficient. If /tmp isn't a local file system\, you have other problems\, and I really don't think this is the same sort of problem on NT.

-- Russ Allbery (rra@​stanford.edu) \<URL​:http​://www.eyrie.org/~eagle/>

[p5pRT]

From [Unknown Contact. See original ticket]

As a fix that is SYSTEM DEPENDENT\,

Big deal. Skate POSIX or die\, to paraphrase Jim Thompson.

use of sysopen with a mode of O_CREAT|O_EXCL will work well\, although only on Unix and only on local filesystems. That's why I didn't send a patch. I'm not sure what the best way of fixing this is (especially in a way that doesn't break all the non-Unix systems).

Perl is always at the mercy of your system libraries (see "man perl" under BUGS)\, and feature-deficient operating systems will be the bane of real programmers forever.

That's no reason not to implement a reasonable fix.

And there *is* a problem. If /tmp/\ is a slink to /etc/passwd or something else that's important\, and root runs it\, you lose. Actually\, there are many other losing scenarios.

Please see my long message of this morning that went also to bugtraq\, to which nary a soul has responded.

--tom

[p5pRT]

From @gsar

On Fri\, 04 Feb 2000 01​:06​:40 PST\, Chip Salzenberg wrote​:

According to Joel Maslak​:

This is due to perldoc not using a secure method to generate a temporary file\, such as mkstemp().

Does this look like a good feature to build into Perl? Temporary files are such a common need\, and they're so easily done wrong.

Nick put in code for open(F\,undef) to do that\, but it didn't survive the interface lawyers. The archives should have all the discussion (there was some good objections\, IIRC).

Sarathy gsar@​ActiveState.com

[p5pRT]

From [Unknown Contact. See original ticket]

Nick put in code for open(F\,undef) to do that\, but it didn't survive the interface lawyers. The archives should have all the discussion (there was some good objections\, IIRC).

I'm having a lovely off-list discussion right now about this entire issue. It looks like we need to write our own tmp{file\,nam}() code\, and add mk*temp*()\, too\, because the POSIX standard doesn't guarantee the needed security\, and many systems (eg. Solaris) seem to do completely the wrong thing​: fopen(tmppath\, "w+").

--tom

[p5pRT]

From [Unknown Contact. See original ticket]

Russ Allbery \rra@&#8203;stanford\.edu wrote

In this particular case\, do we need a temporary file at all? What was the reason for using a temporary file rather than just a pipeline through the translator into the pager? Broken pipe errors from early exits from the pager?

If it was a pipe\, you couldn't go backwards in the pager.

Mike Guy

[p5pRT]

From [Unknown Contact. See original ticket]

M J T Guy \mjtg@&#8203;cus\.cam\.ac\.uk writes​:

Russ Allbery \rra@&#8203;stanford\.edu wrote

In this particular case\, do we need a temporary file at all? What was the reason for using a temporary file rather than just a pipeline through the translator into the pager? Broken pipe errors from early exits from the pager?

If it was a pipe\, you couldn't go backwards in the pager.

You can if you have a decent pager. Hm. Point taken\, though. (I forget how completely brain-dead the default pager is\, given that I switched to less years ago and never looked back.)

-- Russ Allbery (rra@​stanford.edu) \<URL​:http​://www.eyrie.org/~eagle/>

[p5pRT]

From [Unknown Contact. See original ticket]

Russ Allbery \rra@&#8203;stanford\.edu wrote

In this particular case\, do we need a temporary file at all? What was the reason for using a temporary file rather than just a pipeline through the translator into the pager? Broken pipe errors from early exits from the pager?

If it was a pipe\, you couldn't go backwards in the pager.

It it were a good pager\, you could. :-)

--tom

[p5pRT]

From [Unknown Contact. See original ticket]

Simon Cozens writes​:

Would the temp file be unlinked automagically at scope exit?

On co-operating operating systems\, you could do the old sleight-of-hand​: mkstemp\, create the file\, grab the filehandle and unlink the inode from the directory immediately\, dissociating it from the filesystem\, leaving the file open. I don't know what co-operates these days\, though.

If all you need is a tmp *filehandle*\, then there are better ways than creating tmp *files*. If you want a file\, then you cannot unlink it before\, say\, you start the process which will take the name as a parameter.

Ilya

[p5pRT]

From [Unknown Contact. See original ticket]

Simon Cozens writes​:

Would the temp file be unlinked automagically at scope exit?

On co-operating operating systems\, you could do the old sleight-of-hand​: mkstemp\, create the file\, grab the filehandle and unlink the inode from the directory immediately\, dissociating it from the filesystem\, leaving the file open. I don't know what co-operates these days\, though.

If all you need is a tmp *filehandle*\, then there are better ways than creating tmp *files*. If you want a file\, then you cannot unlink it before\, say\, you start the process which will take the name as a parameter.

I am *DELIGHTED* to see that no one has read my important article this morning.

--tom

[p5pRT]

From @chipdude

According to Russ Allbery​:

In this particular case\, do we need a temporary file at all? What was the reason for using a temporary file rather than just a pipeline through the translator into the pager?

Well\, I didn't write the code in question; but if I had to guess\, I'd say it's for systems that don't support real pipes\, e.g. Win32. -- Chip Salzenberg - a.k.a. - \chip@&#8203;valinux\.com "Come back to AT&T!" "We're your friends and family now!" //MST3K