Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

perldoc security bug (race condition) #1135

Closed
p5pRT opened this issue Feb 3, 2000 · 27 comments
Closed

perldoc security bug (race condition) #1135

p5pRT opened this issue Feb 3, 2000 · 27 comments

Comments

@p5pRT
Copy link

p5pRT commented Feb 3, 2000

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

Searchable as RT2095$

@p5pRT
Copy link
Author

p5pRT commented Feb 3, 2000

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
Copy link
Author

p5pRT commented Feb 4, 2000

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
Copy link
Author

p5pRT commented Feb 4, 2000

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
Copy link
Author

p5pRT commented Feb 4, 2000

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
Copy link
Author

p5pRT commented Feb 4, 2000

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
Copy link
Author

p5pRT commented Feb 4, 2000

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
Copy link
Author

p5pRT commented Feb 4, 2000

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
Copy link
Author

p5pRT commented Feb 4, 2000

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
Copy link
Author

p5pRT commented Feb 4, 2000

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
Copy link
Author

p5pRT commented Feb 4, 2000

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
Copy link
Author

p5pRT commented Feb 4, 2000

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
Copy link
Author

p5pRT commented Feb 4, 2000

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
Copy link
Author

p5pRT commented Feb 4, 2000

From [Unknown Contact. See original ticket]

Chip Salzenberg <chip@​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
Copy link
Author

p5pRT commented Feb 4, 2000

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@​valinux.com>
"Come back to AT&T!" "We're your friends and family now!" //MST3K

@p5pRT
Copy link
Author

p5pRT commented Feb 4, 2000

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
Copy link
Author

p5pRT commented Feb 4, 2000

From [Unknown Contact. See original ticket]

Tom Christiansen <tchrist@​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 (<NOTGSGROUP>) {
| 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
Copy link
Author

p5pRT commented Feb 4, 2000

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
Copy link
Author

p5pRT commented Feb 4, 2000

From [Unknown Contact. See original ticket]

Joel Maslak <jmaslak@​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
Copy link
Author

p5pRT commented Feb 4, 2000

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/<your tmpfile> 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
Copy link
Author

p5pRT commented Feb 4, 2000

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
Copy link
Author

p5pRT commented Feb 4, 2000

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
Copy link
Author

p5pRT commented Feb 4, 2000

From [Unknown Contact. See original ticket]

Russ Allbery <rra@​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
Copy link
Author

p5pRT commented Feb 4, 2000

From [Unknown Contact. See original ticket]

M J T Guy <mjtg@​cus.cam.ac.uk> writes​:

Russ Allbery <rra@​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
Copy link
Author

p5pRT commented Feb 4, 2000

From [Unknown Contact. See original ticket]

Russ Allbery <rra@​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
Copy link
Author

p5pRT commented Feb 4, 2000

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
Copy link
Author

p5pRT commented Feb 4, 2000

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
Copy link
Author

p5pRT commented Feb 4, 2000

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@​valinux.com>
"Come back to AT&T!" "We're your friends and family now!" //MST3K

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant