Skip Menu |
Report information
Id: 110730
Status: open
Priority: 0/
Queue: perl5

Owner: Nobody
Requestors: eda [at] waniasset.com
Cc:
AdminCc:

Operating System: Linux
PatchStatus: (no value)
Severity: medium
Type: core
Perl Version: 5.14.2
Fixed In: (no value)



Subject: File::Temp should create tempdir subdirectory whenever possible
Date: Tue, 14 Feb 2012 16:30:39 -0000
To: <perlbug [...] perl.org>
From: "Ed Avis" <eda [...] waniasset.com>
Download (untitled) / with headers
text/plain 6.5k
This is a bug report for perl from eda@waniasset.com, generated with the help of perlbug 1.39 running under perl 5.14.2. ----------------------------------------------------------------- [Please describe your issue here] The secure way to make temporary files is with File::Temp. Sometimes it is necessary to get a temporary file with a known filename to pass it to an external command. Some programs have a default setting to look for libraries or include files in the same directory as the input file. If the temporary filename is directly under /tmp then the program will look in /tmp for include files, which is not secure because anyone can create a file in /tmp with a given name. Since File::Temp already contains code to securely make a temporary directory, the fix is to use that also when returning a temporary file. It should make the directory and the file inside that. Of course, whether this leads to a vulnerability in practice depends on the behaviour of the external command. Many programs do not look in 'the same directory as the input' but only in the current directory or a fixed path. They are not vulnerable. But there are plenty which do. For example, XSLT processors on finding an 'include' directive will look in the same directory as the input file (as mandated by the XSLT standard). Then if using File::Temp to generate a temporary file as input to the XSLT processor, you are vulnerable if an attacker can create a file in /tmp that has the same name as one of the 'include' directives in your file. I am not going to claim this is a vulnerability in Perl itself or that it needs a CVE number or anything dramatic like that. But I feel that File::Temp should do everything possible to make temporary files safe, and minimize the number of potential gotchas. (Can you say off the top of your head whether a given program might look in the directory of its input file for an additional configuration file or any library to load? Certainly, most programmers would not consider the issue when the task at hand is simply 'make a temporary file and give it to program X'.) Since the fix is straightforward, I suggest doing it. I could write a patch if wished. [Please do not change anything below this line] ----------------------------------------------------------------- --- Flags: category=core severity=medium --- This perlbug was built using Perl 5.14.2 in the Fedora build system. It is being executed now by Perl 5.14.2 - Wed Dec 21 12:59:50 UTC 2011. Site configuration information for perl 5.14.2: Configured by Red Hat, Inc. at Wed Dec 21 12:59:50 UTC 2011. Summary of my perl5 (revision 5 version 14 subversion 2) configuration: Platform: osname=linux, osvers=2.6.32-220.el6.x86_64, archname=x86_64-linux-thread-multi uname='linux x86-15.phx2.fedoraproject.org 2.6.32-220.el6.x86_64 #1 smp wed nov 9 08:03:13 est 2011 x86_64 x86_64 x86_64 gnulinux ' config_args='-des -Doptimize=-O2 -g -pipe -Wall -Wp,-D_FORTIFY_SOURCE=2 -fexceptions -fstack-protector --param=ssp-buffer-size=4 -m64 -mtune=generic -Dccdlflags=-Wl,--enable-new-dtags -DDEBUGGING=-g -Dversion=5.14.2 -Dmyhostname=localhost -Dperladmin=root@localhost -Dcc=gcc -Dcf_by=Red Hat, Inc. -Dprefix=/usr -Dvendorprefix=/usr -Dsiteprefix=/usr/local -Dsitelib=/usr/local/share/perl5 -Dsitearch=/usr/local/lib64/perl5 -Dprivlib=/usr/share/perl5 -Dvendorlib=/usr/share/perl5/vendor_perl -Darchlib=/usr/lib64/perl5 -Dvendorarch=/usr/lib64/perl5/vendor_perl -Darchname=x86_64-linux-thread-multi -Dlibpth=/usr/local/lib64 /lib64 /usr/lib64 -Duseshrplib -Dusethreads -Duseithreads -Dusedtrace=/usr/bin/dtrace -Duselargefiles -Dd_semctl_semun -Di_db -Ui_ndbm -Di_gdbm -Di_shadow -Di_syslog -Dman3ext=3pm -Duseperlio -Dinstallusrbinperl=n -Ubincompat5005 -Uversiononly -Dpager=/usr/bin/less -isr -Dd_gethostent_r_proto -Ud_endhostent_r_proto -Ud_sethostent_r_proto -Ud_endprotoent_r_proto -Ud_setprotoent_r_proto -Ud_endservent_r_proto -Ud_setservent_r_proto -Dscriptdir=/usr/bin' hint=recommended, useposix=true, d_sigaction=define useithreads=define, usemultiplicity=define useperlio=define, d_sfio=undef, uselargefiles=define, usesocks=undef use64bitint=define, use64bitall=define, uselongdouble=undef usemymalloc=n, bincompat5005=undef Compiler: cc='gcc', ccflags ='-D_REENTRANT -D_GNU_SOURCE -fno-strict-aliasing -pipe -fstack-protector -I/usr/local/include -D_LARGEFILE_SOURCE -D_FILE_OFFSET_BITS=64', optimize='-O2 -g -pipe -Wall -Wp,-D_FORTIFY_SOURCE=2 -fexceptions -fstack-protector --param=ssp-buffer-size=4 -m64 -mtune=generic', cppflags='-D_REENTRANT -D_GNU_SOURCE -fno-strict-aliasing -pipe -fstack-protector -I/usr/local/include' ccversion='', gccversion='4.6.2 20111027 (Red Hat 4.6.2-1)', gccosandvers='' intsize=4, longsize=8, ptrsize=8, doublesize=8, byteorder=12345678 d_longlong=define, longlongsize=8, d_longdbl=define, longdblsize=16 ivtype='long', ivsize=8, nvtype='double', nvsize=8, Off_t='off_t', lseeksize=8 alignbytes=8, prototype=define Linker and Libraries: ld='gcc', ldflags =' -fstack-protector' libpth=/usr/local/lib64 /lib64 /usr/lib64 libs=-lresolv -lnsl -lgdbm -ldb -ldl -lm -lcrypt -lutil -lpthread -lc -lgdbm_compat perllibs=-lresolv -lnsl -ldl -lm -lcrypt -lutil -lpthread -lc libc=, so=so, useshrplib=true, libperl=libperl.so gnulibc_version='2.14.90' Dynamic Linking: dlsrc=dl_dlopen.xs, dlext=so, d_dlsymun=undef, ccdlflags='-Wl,--enable-new-dtags -Wl,-rpath,/usr/lib64/perl5/CORE' cccdlflags='-fPIC', lddlflags='-shared -O2 -g -pipe -Wall -Wp,-D_FORTIFY_SOURCE=2 -fexceptions -fstack-protector --param=ssp-buffer-size=4 -m64 -mtune=generic' Locally applied patches: --- @INC for perl 5.14.2: /usr/local/lib64/perl5 /usr/local/share/perl5 /usr/lib64/perl5/vendor_perl /usr/share/perl5/vendor_perl /usr/lib64/perl5 /usr/share/perl5 . --- Environment for perl 5.14.2: HOME=/home/eda LANG=en_GB.UTF-8 LANGUAGE (unset) LC_COLLATE=C LC_CTYPE=en_GB.UTF-8 LC_MESSAGES=en_GB.UTF-8 LC_MONETARY=en_GB.UTF-8 LC_NUMERIC=en_GB.UTF-8 LC_TIME=en_GB.UTF-8 LD_LIBRARY_PATH (unset) LOGDIR (unset) PATH=/home/eda/bin:/home/eda/bin:/usr/local/bin:/bin:/usr/bin:/sbin:/usr /sbin:/sbin:/usr/sbin PERL_BADLANG (unset) SHELL=/bin/bash -- Ed Avis <eda@waniasset.com> Show quoted text
______________________________________________________________________ This email has been scanned by the Symantec Email Security.cloud service. For more information please visit http://www.symanteccloud.com
______________________________________________________________________
Subject: Re: [perl #110730] File::Temp should create tempdir subdirectory whenever possible
Date: Tue, 14 Feb 2012 17:05:34 +0000 (UTC)
To: perl5-porters [...] perl.org
From: Petr Pisar <ppisar [...] redhat.com>
Download (untitled) / with headers
text/plain 544b
On 2012-02-14, Ed Avis <perlbug-followup@perl.org> wrote: Show quoted text
> > I am not going to claim this is a vulnerability in Perl itself or that > it needs a CVE number or anything dramatic like that. But I feel that > File::Temp should do everything possible to make temporary files safe, > and minimize the number of potential gotchas.
Actually this or similar issue is already tracked on CPAN <https://rt.cpan.org/Public/Bug/Display.html?id=69106> and it has been assigned CVE-2011-4116 <https://bugzilla.redhat.com/show_bug.cgi?id=753970>. -- Petr
Subject: Re: [perl #110730] File::Temp should create tempdir subdirectory whenever possible
Date: Wed, 15 Feb 2012 09:35:27 +0000 (UTC)
To: perl5-porters [...] perl.org
From: Ed Avis <eda [...] waniasset.com>
Download (untitled) / with headers
text/plain 1.1k
Petr Pisar <ppisar <at> redhat.com> writes: Show quoted text
>Actually this or similar issue is already tracked on CPAN ><https://rt.cpan.org/Public/Bug/Display.html?id=69106>
I believe that is a different issue. There, if you pass your own template string (containing some XXX to be replaced to make a unique name) but the template string refers to a subdirectory of /tmp, you are vulnerable to an attacker creating that subdirectory. What I'm referring to is that if you just call tempfile() using the default template string, the temporary file is indeed created securely, but it can lead to unsafe practices because if you pass a filename /tmp/abc to an external command, that external command might have logic to look in the parent directory of /tmp/abc to find libraries or include files. In other words, the command you ran will load files from /tmp, and they can be placed there by an attacker. The fix is for File::Temp to always make a subdirectory and return /tmp/abc/def as the temporary file name from tempfile(). Then any external command would only look in /tmp/abc for other files, which is safe because /tmp/abc is newly created and without permissions for other users. -- Ed Avis <eda@waniasset.com>
Subject: Re: [perl #110730] File::Temp should create tempdir subdirectory whenever possible
Date: Wed, 15 Feb 2012 13:44:28 +0000 (UTC)
To: perl5-porters [...] perl.org
From: Petr Pisar <ppisar [...] redhat.com>
On 2012-02-15, Ed Avis <eda@waniasset.com> wrote: Show quoted text
> Petr Pisar <ppisar <at> redhat.com> writes: >
>>Actually this or similar issue is already tracked on CPAN >><https://rt.cpan.org/Public/Bug/Display.html?id=69106>
> > I believe that is a different issue. >
You are right. Show quoted text
> What I'm referring to is that if you just call tempfile() using the > default template string, the temporary file is indeed created > securely, but it can lead to unsafe practices because if you pass a > filename /tmp/abc to an external command, that external command might > have logic to look in the parent directory of /tmp/abc to find > libraries or include files.
Well, external tool can have a logic to traverse all parent directories or whatever. I think this is problem of the external tool and its input. I heard about securing TeX processor. They used chrooted environment for that purpose. You need to know which risks the external tool brings, then you can mittigate them. So I think creating secure files nested into secure directory by default is only specific solution for specific problem. -- Petr
Subject: Re: [perl #110730] File::Temp should create tempdir subdirectory whenever possible
Date: Thu, 16 Feb 2012 09:49:33 +0000 (UTC)
To: perl5-porters [...] perl.org
From: Ed Avis <eda [...] waniasset.com>
Download (untitled) / with headers
text/plain 2.4k
Petr Pisar <ppisar <at> redhat.com> writes: Show quoted text
>>What I'm referring to is that if you just call tempfile() using the >>default template string, the temporary file is indeed created >>securely, but it can lead to unsafe practices because if you pass a >>filename /tmp/abc to an external command, that external command might >>have logic to look in the parent directory of /tmp/abc to find >>libraries or include files.
Show quoted text
>I think this is problem of the external tool and its input.
In principle, yes. But I am considering how to make temporary file usage more secure in practice. Can you say from memory whether a given program looks in the directory of its input or not? Some do, some don't, and there is no consensus that doing so is a bug. (In some cases, such as XSLT, it is mandated by the relevant standards.) So while I agree that in a perfect world somebody would audit all programs and fix them not to use the input file's directory, right now there is a trap for the unwary. Since the fix is straightforward we should just do it. It's the same consideration with the filename generated. Why does File::Temp only generate temporary filenames with A-Za-z0-9 characters? Surely it would be fine on Unix-like systems to generate them with any 8-bit character except / and \0 - and if some external tool misbehaves, that is a problem of the external tool. Again, true in principle but dangerous in practice. Temporary files are hard to get right even for experienced programmers. (In this particular case I had the vulnerability in my code for a long time before realizing it.) File::Temp is there to provide a way to make temp files securely. It should do everything possible to provide a simple interface that is hard to use wrongly, even for inexperienced programmers. Show quoted text
>So I think creating secure files nested into secure >directory by default is only specific solution for specific problem.
I think it is a pretty common idiom to say my ($fh, $filename) = tempfile(); system 'some', 'command', $filename; If it were just some weird one-off case I would not suggest any fix, but many programs do things like the above. And you are right that in principle an external command could do all sorts of stupid things like looking in ../../../config for files to load - but that is really an unusual and uncommon case. By contrast, looking in the same directory as the input file is pretty common and easily exploitable in /tmp. -- Ed Avis <eda@waniasset.com>
Subject: Re: [perl #110730] File::Temp should create tempdir subdirectory whenever possible
Date: Thu, 16 Feb 2012 10:40:53 +0000 (UTC)
To: perl5-porters [...] perl.org
From: Petr Pisar <ppisar [...] redhat.com>
Download (untitled) / with headers
text/plain 1.6k
On 2012-02-16, Ed Avis <eda@waniasset.com> wrote: Show quoted text
> Petr Pisar <ppisar <at> redhat.com> writes: >
>>>What I'm referring to is that if you just call tempfile() using the >>>default template string, the temporary file is indeed created >>>securely, but it can lead to unsafe practices because if you pass a >>>filename /tmp/abc to an external command, that external command might >>>have logic to look in the parent directory of /tmp/abc to find >>>libraries or include files.
[...] Show quoted text
>>So I think creating secure files nested into secure >>directory by default is only specific solution for specific problem.
> > I think it is a pretty common idiom to say > > my ($fh, $filename) = tempfile(); > system 'some', 'command', $filename; > > If it were just some weird one-off case I would not suggest any fix, > but many programs do things like the above. And you are right that in > principle an external command could do all sorts of stupid things like > looking in ../../../config for files to load - but that is really an > unusual and uncommon case. By contrast, looking in the same directory > as the input file is pretty common and easily exploitable in /tmp. >
I don't remember whole XSLT specification, but isn't this problem just plain relative path resolution issue? Then you need to sanity the XML input first. And actually XML itself can contain XInclude or DTD references. With relative paths like '../foo'. What depth of secure subdirectories do you consider as safe? One? Two? I don't share your estimation on weird one-off and pretty common numbers. Thus instead of modifying tempfile(), I would add new function or new tempfile() parameter to create nested path explicitly with explicit dept level. -- Petr


This service is sponsored and maintained by Best Practical Solutions and runs on Perl.org infrastructure.

For issues related to this RT instance (aka "perlbug"), please contact perlbug-admin at perl.org