Bug #72862 for Proc-ProcessTable: unsafe use of /tmp

Thu Dec 01 01:24:10 2011 CARNIL [...] cpan.org - Ticket created

From:	CARNIL [...] cpan.org
Subject:	unsafe use of /tmp

Hi This bug has been forwarded from http://bugs.debian.org/650500 Package: libproc-processtable-perl Version: 0.45-1 Severity: important Tags: security Proc::ProcessTable can cache TTY information (not enabled by default). For this it uses the file /tmp/TTYDEVS. If caching is enabled, there is a race condition that allows to overwrite arbitrary files in ProcessTable.pm: 102 if( -r $TTYDEVSFILE ) 103 { 104 $_ = Storable::retrieve($TTYDEVSFILE); [...] 107 else 108 { [...] 112 Storable::store(\%Proc::ProcessTable::TTYDEVS, $TTYDEVSFILE); If a symlink /tmp/TTYDEVS is created between line 102 and 112, the file the link points to is overwritten. Alternatively wrong information can be provided. The relevant code path can be reached with perl -MProc::ProcessTable -e 'my $t = Proc::ProcessTable->new(cache_ttys => 1, enable_ttys => 1); $t->table;' Ansgar References: [1] http://bugs.debian.org/650500 [2] https://bugzilla.redhat.com/show_bug.cgi?id=CVE-2011-4363 Thanks in advance, Salvatore Bonaccorso, Debian Perl Group

Tue Jun 26 14:36:17 2012 intrigeri [...] boum.org - Correspondence added

Ping? Any chance this security issue is addressed?

Tue Jun 26 14:36:18 2012 The RT System itself - Status changed from 'new' to 'open'

Wed Jun 27 01:18:17 2012 REHSACK [...] cpan.org - Correspondence added

This will be fixed, but with a completely different solution. Thanks for reporting, Jens

Sat Oct 27 21:27:03 2012 ether [...] cpan.org - Cc ETHER added

Sun Oct 28 15:31:41 2012 ether [...] cpan.org - Severity Important added

Fri Nov 09 13:13:14 2012 cpan [...] bargsten.org - Correspondence added

Would this be an appropriate patch?

Subject:

tmp_ttydevs_file_security_fix.patch

diff --git a/ProcessTable.pm b/ProcessTable.pm index 03b741d..d3574ba 100644 --- a/ProcessTable.pm +++ b/ProcessTable.pm @@ -4,6 +4,7 @@ use 5.006; use strict; use Carp; +use Fcntl; use vars qw($VERSION @ISA @EXPORT @EXPORT_OK $AUTOLOAD); require Exporter; @@ -109,7 +110,12 @@ sub initialize $self->_get_tty_list; my $old_umask = umask; umask 022; - Storable::store(\%Proc::ProcessTable::TTYDEVS, $TTYDEVSFILE); + + sysopen( my $ttydevs_fh, $TTYDEVSFILE, O_WRONLY | O_EXCL | O_CREAT ) + or die "$TTYDEVSFILE was created by other process"; + Storable::store_fd( \%Proc::ProcessTable::TTYDEVS, $ttydevs_fh ); + close $ttydevs_fh; + umask $old_umask; } }

Wed Feb 06 17:33:46 2013 SREZIC [...] cpan.org - Correspondence added

On 2012-11-09 13:13:14, jwb wrote: Show quoted text

> Would this be an appropriate patch?

Another problem is that different perl versions with different byte orders (i.e. a 32bit perl vs. a 64bit perl) would still use the same file name, but loading the Storable data would fail then. See https://rt.cpan.org/Ticket/Display.html?id=37722 So /tmp/TTYDEVS should possibly have the byteorder in the filename, too (i.e. /tmp/TTYDEVS_1234 or /tmp/TTYDEVS_12345678 or /tmp/TTYDEVS_87654321 ...) Regards, Slaven

Sat Feb 16 14:29:47 2013 cpan [...] bargsten.org - Correspondence added

fixed v0.47, the TTYDEVSFILE issue is fixed in github ( https:// github.com/jwbargsten/perl-proc-processtable ) and will be part of next release.

Sat Feb 16 14:29:49 2013 cpan [...] bargsten.org - Status changed from 'open' to 'resolved'