search

Perl / perl5

🐪 The Perl programming language
https://dev.perl.org/perl5/
Other
2k stars 559 forks source link

Test failure in lib/ftmp-security.t #2381

Closed p5pRT closed 21 years ago

p5pRT commented 24 years ago

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

Searchable as RT3720$

p5pRT commented 24 years ago

From urban@ast.lmco.com

Created by urban@spica.ast.lmco.com

Below is the output from running ./perl harness.

lib/ftmp-security...ok 3/13File​::Temp​::_gettemp​: Parent directory (./) is not safe (sticky bit not set when world writable?) at lib/ftmp-security.t line 97 Error in tempfile() using ./temptestXXXXXXXX at lib/ftmp-security.t line 97 lib/ftmp-security...dubious
  Test returned status 255 (wstat 65280\, 0xff00) DIED. FAILED tests 6-13   Failed 8/13 tests\, 38.46% okay

This is the output from running ./perl lib/ftmp-security.t :

1..13 ok 1 # Testing with STANDARD security... # Fname1 = ./temptestYCPjeLht ok 2 ok 3 # Testing with MEDIUM security... File​::Temp​::_gettemp​: Parent directory (./) is not safe (sticky bit not set when world writable?) at lib/ftmp-security.t line 97 Error in tempfile() using ./temptestXXXXXXXX at lib/ftmp-security.t line 97 ok 4 ok 5

I looked in the file but there were no specific warnings or comments about this test on HP's.

I am trying to learn about the perl internals and any suggestions at what to look at for this test and what direction I could proceed for developing a patch make work correctly on a HP would be greatly appreciated.

Perl Info ``` Flags: category=core severity=low Site configuration information for perl v5.7.0: Configured by urban at Wed Aug 16 09:58:12 MDT 2000. Summary of my perl5 (revision 5.0 version 7 subversion 0) configuration: Platform: osname=hpux, osvers=10.20, archname=PA-RISC1.1 uname='hp-ux spica b.10.20 a 9000735 2008140532 two-user license ' config_args='-Accflags=-DPERL_POLLUTE -Dcc=gcc -Dprefix=/disk2/perl' hint=recommended, useposix=true, d_sigaction=define usethreads=undef use5005threads=undef useithreads=undef usemultiplicity=undef useperlio=undef d_sfio=undef uselargefiles=undef use64bitint=undef use64bitall=undef uselongdouble=undef usesocks=undef Compiler: cc='gcc', optimize='-O', gccversion=2.96 20000703 (experimental), gccosandvers=hpux10.20 cppflags='-D_HPUX_SOURCE -L/lib/pa1.1 -DUINT32_MAX_BROKEN -DPERL_POLLUTE -fno-strict-aliasing -I/home/urban/local/hp/include' ccflags ='-D_HPUX_SOURCE -L/lib/pa1.1 -DUINT32_MAX_BROKEN -DPERL_POLLUTE -fno-strict-aliasing -I/home/urban/local/hp/include' stdchar='unsigned char', d_stdstdio=define, usevfork=false intsize=4, longsize=4, ptrsize=4, doublesize=8 d_longlong=define, longlongsize=8, d_longdbl=define, longdblsize=16 ivtype='long', ivsize=4, nvtype='double', nvsize=8, Off_t='off_t', lseeksize=4 alignbytes=8, usemymalloc=y, prototype=define Linker and Libraries: ld='ld', ldflags ='-L/usr/local/lib -L/home/urban/local/hp/lib' libpth=/usr/local/lib /lib /usr/lib /usr/ccs/lib /home/urban/local/hp/lib libs=-lnsl_s -lndbm -lgdbm -ldld -lm -lc -lndir -lcrypt -lsec libc=/lib/libc.sl, so=sl, useshrplib=false, libperl=libperl.a Dynamic Linking: dlsrc=dl_hpux.xs, dlext=sl, d_dlsymun=undef, ccdlflags='-Wl,-E -Wl,-B,deferred ' cccdlflags='-fpic', lddlflags='-b -L/usr/local/lib -L/home/urban/local/hp/lib' Locally applied patches: SUIDMAIL - fixes for suidperl security DEVEL6654 @INC for perl v5.7.0: lib /disk2/perl/lib/5.7.0/PA-RISC1.1 /disk2/perl/lib/5.7.0 /disk2/perl/lib/site_perl/5.7.0/PA-RISC1.1 /disk2/perl/lib/site_perl/5.7.0 /disk2/perl/lib/site_perl . Environment for perl v5.7.0: HOME=/home/urban LANG=C LANGUAGE (unset) LD_LIBRARY_PATH (unset) LOGDIR (unset) PATH=/home/urban/local/hp/bin:/home/urban/local/hp/bin/X11:/appl/sqa/bin:/opt/langtools/bin:/usr/ccs/bin:/opt/softbench/bin:/appl/share/kms:/home/urban/bin:/usr/local/bin:/usr/local/sbin:/usr/local/share/bin:/usr/local/share/sbin:/usr/local/share/etc:/usr/dt/bin:/usr/bin/X11:/usr/contrib/bin/X11:/usr/local/bin/X11:/bin:/usr/bin:/usr/sbin:/etc:/usr/etc:/usr/local/share/bin/hosts:.:/opt/imake/bin:.:/usr/icc750_i960/bin:/usr/icc750_i960/rel:/appl/share/bin:/project/msls_tools:/project/msls_tools/proc_sched PERL_BADLANG (unset) SHELL=/bin/csh SHLIB_PATH (unset) ```
p5pRT commented 24 years ago

From @timj

On Wed\, 16 Aug 2000\, 'David Scott Urban wrote​:

This is a bug report for perl from urban@​spica.ast.lmco.com\, generated with the help of perlbug 1.31 running under perl v5.7.0.

----------------------------------------------------------------- [Please enter your report here] Below is the output from running ./perl harness.

lib/ftmp-security...ok 3/13File​::Temp​::_gettemp​: Parent directory (./) is not safe (sticky bit not set when world writable?) at lib/ftmp-security.t line 97 Error in tempfile() using ./temptestXXXXXXXX at lib/ftmp-security.t line 97 lib/ftmp-security...dubious
Test returned status 255 (wstat 65280\, 0xff00) DIED. FAILED tests 6-13 Failed 8/13 tests\, 38.46% okay

This is the output from running ./perl lib/ftmp-security.t :

1..13 ok 1 # Testing with STANDARD security... # Fname1 = ./temptestYCPjeLht ok 2 ok 3 # Testing with MEDIUM security... File​::Temp​::_gettemp​: Parent directory (./) is not safe (sticky bit not set when world writable?) at lib/ftmp-security.t line 97 Error in tempfile() using ./temptestXXXXXXXX at lib/ftmp-security.t line 97 ok 4 ok 5

Out of interest\, was the directory you were running in world writable?

% ls -l .

I looked in the file but there were no specific warnings or comments about this test on HP's.

I am trying to learn about the perl internals and any suggestions at what to look at for this test and what direction I could proceed for developing a patch make work correctly on a HP would be greatly appreciated.

The MEDIUM security test simply checks to see if the directory you wish to place the file in is safe. If the directory is only writable by you then no problem. If the directory is world writable it checks that the sticky bit is set.

The _is_safe subroutine in File​::Temp does this test​:

  # Check to see whether owner is neither superuser (or a system uid) nor me   # Use the real uid from the $\< variable   # UID is in [4]   if ( $info[4] > File​::Temp->top_system_uid() && $info[4] != $\<) {   carp "Directory owned neither by root nor the current user";   return 0;   }

  # check whether group or other can write file   # use 066 to detect either reading or writing   # use 022 to check writability   # Do it with S_IWOTH and S_IWGRP for portability (maybe)   # mode is in info[2]   if (($info[2] & &Fcntl​::S_IWGRP) || # Is group writable?   ($info[2] & &Fcntl​::S_IWOTH) ) { # Is world writable?   return 0 unless -d _; # Must be a directory   return 0 unless -k _; # Must be sticky   }

I'm assuming HPUX recognizes stickyness? If this test is not true you will get the error.

The strange thing is that it passes the HIGH security test. That test effectively runs _is_safe for all the direcories above you (including the current). I do not understand how MEDIUM fails but HIGH passes.

p5pRT commented 24 years ago

From @timj

On Wed\, 16 Aug 2000\, Tim Jenness wrote​:

On Wed\, 16 Aug 2000\, 'David Scott Urban wrote​:

This is a bug report for perl from urban@​spica.ast.lmco.com\, generated with the help of perlbug 1.31 running under perl v5.7.0.

----------------------------------------------------------------- [Please enter your report here] Below is the output from running ./perl harness.

lib/ftmp-security...ok 3/13File​::Temp​::_gettemp​: Parent directory (./) is not safe (sticky bit not set when world writable?) at lib/ftmp-security.t line 97 Error in tempfile() using ./temptestXXXXXXXX at lib/ftmp-security.t line 97 lib/ftmp-security...dubious
Test returned status 255 (wstat 65280\, 0xff00) DIED. FAILED tests 6-13 Failed 8/13 tests\, 38.46% okay

This is the output from running ./perl lib/ftmp-security.t :

1..13 ok 1 # Testing with STANDARD security... # Fname1 = ./temptestYCPjeLht ok 2 ok 3 # Testing with MEDIUM security... File​::Temp​::_gettemp​: Parent directory (./) is not safe (sticky bit not set when world writable?) at lib/ftmp-security.t line 97 Error in tempfile() using ./temptestXXXXXXXX at lib/ftmp-security.t line 97 ok 4 ok 5

\

The strange thing is that it passes the HIGH security test. That test effectively runs _is_safe for all the direcories above you (including the current). I do not understand how MEDIUM fails but HIGH passes.

Sorry. I was being stupid and had not spotted that only 5 tests came back when it was run by the hand. The problem is that MEDIUM fails with a croak so that HIGH is never tried. The last two tests are clearing up after tests 2 and 3 (via END).

Sorry for the confusion.

p5pRT commented 24 years ago

From [Unknown Contact. See original ticket]

Today around 3​:33pm\, Tim Jenness hammered out this masterpiece​:

: : > The strange thing is that it passes the HIGH security test. That test : > effectively runs _is_safe for all the direcories above you (including the : > current). I do not understand how MEDIUM fails but HIGH passes. : > : : Sorry. I was being stupid and had not spotted that only 5 tests came back : when it was run by the hand. The problem is that MEDIUM fails with a croak

Is that a new government operation?

: so that HIGH is never tried. The last two tests are clearing up after : tests 2 and 3 (via END). : : Sorry for the confusion.

No problem :-)

p5pRT commented 24 years ago

From @timj

On Thu\, 17 Aug 2000\, 'David Scott Urban wrote​:

Out of interest\, was the directory you were running in world writable?

% ls -l .

No\, my directories are set with a umask 002.

Here is tha output from ls -la on the t directory​:

Thanks.

I see that they are group writable though. This is the problem. File​::Temp thinks that a directory that is writable by anyone other than you or root is unsafe unless the sticky bit is set. I can recreate your error on linux by setting g+w on the directory.

I'm really not sure what to do next though. Is the test on group writability ridiculous or justifiably paranoid. At this point I'm not sure how to deal with the case of umask 002. I could change the test to use tmpdir() rather than current directory or I could check group writeability on current before porceeding with the test OR I could remove the check on group writability from the module.

Comments?

[spica] ~/perl/perl/t 309> ls -la total 136 drwxrwsr-x 10 urban csupport 4096 Aug 16 15​:38 ./ drwxrwsr-x 28 urban csupport 8192 Aug 16 10​:40 ../ -r--r--r-- 1 urban csupport 728 Aug 13 12​:36 README -r-xr-xr-x 1 urban csupport 4341 Aug 13 12​:36 TEST* -r-xr-xr-x 1 urban csupport 4584 Aug 13 12​:36 UTEST* drwxrwsr-x 2 urban csupport 4096 Aug 16 09​:43 base/ drwxrwsr-x 2 urban csupport 4096 Aug 16 09​:43 cmd/ drwxrwsr-x 2 urban csupport 4096 Aug 16 09​:43 comp/ -r--r--r-- 1 urban csupport 1788 Aug 13 12​:36 harness drwxrwsr-x 2 urban csupport 4096 Aug 16 09​:43 io/ drwxrwsr-x 3 urban csupport 4096 Aug 16 15​:37 lib/ drwxrwsr-x 2 urban csupport 4096 Aug 16 15​:31 op/ lrwxrwxrwx 1 urban csupport 7 Aug 16 10​:57 perl -> ../perl* drwxrwsr-x 2 urban csupport 4096 Aug 16 09​:44 pod/ drwxrwsr-x 4 urban csupport 4096 Aug 16 09​:44 pragma/

p5pRT commented 24 years ago

From [Unknown Contact. See original ticket]

Thanks.

I see that they are group writable though. This is the problem. File​::Temp thinks that a directory that is writable by anyone other than you or root is unsafe unless the sticky bit is set. I can recreate your error on linux by setting g+w on the directory.

I'm really not sure what to do next though. Is the test on group writability ridiculous or justifiably paranoid. At this point I'm not sure how to deal with the case of umask 002. I could change the test to use tmpdir() rather than current directory or I could check group writeability on current before porceeding with the test OR I could remove the check on group writability from the module.

Comments?

After some serious thought\, I would trust the people in the groups that I work with to do the right thing. I definately would not make things world writeable.

I don't think the test on writability is ridiculous nor paranoid. It is whether you have the faith that people in the same group know what they are doing.

D. S. Urban
email : urban@​ast.lmco.com

To be the person\, you must know the person. To know the person\, you must understand the person. To understand the person\, you must listen. To listen\, you must open your mind and put aside all preconceived ideas and notions.

All opinions expressed are my own not that of my employer

p5pRT commented 24 years ago

From @floatingatoll

I'm really not sure what to do next though. Is the test on group writability ridiculous or justifiably paranoid. At this point I'm not sure how to deal with the case of umask 002. I could change the test to use tmpdir() rather than current directory or I could check group writeability on current before porceeding with the test OR I could remove the check on group writability from the module.

Comments?

Perhaps we should create a directory structure during the test with various security issues and make sure the module errors correctly under those circumstances\, instead of depending on the specific permissions of a system for the pass/fail of a test. This isn't a test for security holes on your system\, this is a test for functionality of a module.

R.

p5pRT commented 24 years ago

From @timj

On Fri\, 18 Aug 2000\, Richard Soderberg wrote​:

I'm really not sure what to do next though. Is the test on group writability ridiculous or justifiably paranoid. At this point I'm not sure how to deal with the case of umask 002. I could change the test to use tmpdir() rather than current directory or I could check group writeability on current before porceeding with the test OR I could remove the check on group writability from the module.

Comments?

Perhaps we should create a directory structure during the test with various security issues and make sure the module errors correctly under those circumstances\, instead of depending on the specific permissions of a system for the pass/fail of a test. This isn't a test for security holes on your system\, this is a test for functionality of a module.

That's still quite difficult since in principal File​::Temp checks whether all the parent directories are safe as well. That means that you could make a directory somewhere and set the protections in a controlled\, cross platform\, way but if the directory you create is itself in an unsafe directory then you still can't do the tests :-(

The only thing I can think of is to move the test directory to tmpdir rather than cwd - can we guarantee that tmpdir is going to be "safe" more often than cwd?

p5pRT commented 24 years ago

From @jhi

On Fri\, Aug 18\, 2000 at 06​:23​:34PM -0700\, Richard Soderberg wrote​:

I'm really not sure what to do next though. Is the test on group writability ridiculous or justifiably paranoid. At this point I'm not sure how to deal with the case of umask 002. I could change the test to use tmpdir() rather than current directory or I could check group writeability on current before porceeding with the test OR I could remove the check on group writability from the module.

Comments?

Perhaps we should create a directory structure during the test with various security issues and make sure the module errors correctly under those circumstances\, instead of depending on the specific permissions of a system

That sounds like a good idea. And if we created that directory structure at File​::Spec->tmpdir (which\, hopefully\, is a local filesystem) we could get a success for that one File​::Temp test which always fails on NFS.

for the pass/fail of a test. This isn't a test for security holes on your system\, this is a test for functionality of a module.

R.

p5pRT commented 24 years ago

From @timj

On Sat\, 19 Aug 2000\, Jarkko Hietaniemi wrote​:

On Fri\, Aug 18\, 2000 at 06​:23​:34PM -0700\, Richard Soderberg wrote​:

I'm really not sure what to do next though. Is the test on group writability ridiculous or justifiably paranoid. At this point I'm not sure how to deal with the case of umask 002. I could change the test to use tmpdir() rather than current directory or I could check group writeability on current before porceeding with the test OR I could remove the check on group writability from the module.

Comments?

Perhaps we should create a directory structure during the test with various security issues and make sure the module errors correctly under those circumstances\, instead of depending on the specific permissions of a system

That sounds like a good idea. And if we created that directory structure at File​::Spec->tmpdir (which\, hopefully\, is a local filesystem) we could get a success for that one File​::Temp test which always fails on NFS.

Okay. For the security tests I'll change them to use tmpdir(). Note that as mentioned in a later email that this still assumes that tmpdir() itself is "safe" (and\, possibly\, all its parent directories). - I can't see anyway around that though...

Here it is​:

Inline Patch ```diff --- t/lib/ftmp-security.t.ori Sat Aug 19 10:20:23 2000 +++ t/lib/ftmp-security.t Sat Aug 19 10:20:57 2000 @@ -95,7 +95,7 @@ # Create the tempfile my $template = "temptestXXXXXXXX"; my ($fh1, $fname1) = tempfile ( $template, - DIR => File::Spec->curdir, + DIR => File::Spec->tmpdir, UNLINK => 1, ); print "# Fname1 = $fname1\n"; ```