File::Temp should create tempdir subdirectory whenever possible

[p5pRT]

Migrated from rt.perl.org#110730 (status was 'open')

Searchable as RT110730$

[p5pRT]

From @epa

Created by @epa

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.

Perl Info

``` 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 ______________________________________________________________________ This email has been scanned by the Symantec Email Security.cloud service. For more information please visit http://www.symanteccloud.com ______________________________________________________________________ ```

[p5pRT]

From @ppisar

On 2012-02-14\, Ed Avis \perlbug\-followup@​perl\.org wrote​:

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

[p5pRT]

The RT System itself - Status changed from 'new' to 'open'

[p5pRT]

From @epa

Petr Pisar \<ppisar \ 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. 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@&#8203;waniasset\.com

[p5pRT]

From @ppisar

On 2012-02-15\, Ed Avis \eda@&#8203;waniasset\.com wrote​:

Petr Pisar \<ppisar \ 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.

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

[p5pRT]

From @epa

Petr Pisar \<ppisar \ 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.

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.

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@&#8203;waniasset\.com

[p5pRT]

From @ppisar

On 2012-02-16\, Ed Avis \eda@&#8203;waniasset\.com wrote​:

Petr Pisar \<ppisar \ 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. [...] 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