From @steve-m-hay

Created by SteveHay@planit.com

Run the following program under perl 5.10.0 on Windows XP​:

#!perl -wT
use strict;
use warnings;
BEGIN { $ENV{TMPDIR} = "$ENV{WINDIR}\\TEMP" };
use CGI;
my $tmpfile = new CGITempFile(1);
print "tmpfile='", $tmpfile->as_string(), "'\n";

This causes the error​:

Insecure dependency in sprintf while running with -T switch at (eval 2)
line 6.

Under perl 5.8.8 it runs fine​:

tmpfile='C​:\WINDOWS\TEMP\CGItemp1'

The reason is the following item listed in perl5100delta​:

"When perl is run under taint mode, printf() and sprintf() will now
reject any tainted format argument."

This causes a problem for CGI.pm, because CGITempFile​::find_tempdir()
builds a hard-coded list of candidate temporary directories, but then
says​:

unshift(@​TEMP,$ENV{'TMPDIR'}) if defined $ENV{'TMPDIR'};

so this *tainted* candidate is the first one to be tried and is hence
selected as the value of $TMPDIRECTORY since in this case it happens to
exist.

The above error is then generated by CGITempFile​::new when it uses this
*tainted* value in an sprintf() to build the temporary file name​:

last if ! -f ($filename =
sprintf("${TMPDIRECTORY}${SL}CGItemp%d",$sequence++));

Obviously there are various ways around this (it's currently causing me
trouble with a Bugzilla system running on 5.10.0, and I've worked around
the problem by just commenting-out the unshift() line above so that the
tainted value is not a candidate), but I'm not sure what a good
permanent fix would be.

Flags:
    category=library
    severity=medium

Site configuration information for perl 5.10.0:

Configured by SYSTEM at Thu Jan 10 11:00:30 2008.

Summary of my perl5 (revision 5 version 10 subversion 0) configuration:
  Platform:
    osname=MSWin32, osvers=5.00, archname=MSWin32-x86-multi-thread
    uname=''
    config_args='undef'
    hint=recommended, useposix=true, d_sigaction=undef
    useithreads=define, usemultiplicity=define
    useperlio=define, d_sfio=undef, uselargefiles=define, usesocks=undef
    use64bitint=undef, use64bitall=undef, uselongdouble=undef
    usemymalloc=n, bincompat5005=undef
  Compiler:
    cc='cl', ccflags ='-nologo -GF -W3 -MD -Zi -DNDEBUG -O1 -DWIN32
-D_CONSOLE -DNO_STRICT -DHAVE_DES_FCRYPT -DUSE_SITECUSTOMIZE
-DPRIVLIB_LAST_IN_INC -DPERL_IMPLICIT_CONTEXT -DPERL_IMPLICIT_SYS
-DUSE_PERLIO -DPERL_MSVCRT_READFIX',
    optimize='-MD -Zi -DNDEBUG -O1',
    cppflags='-DWIN32'
    ccversion='14.00.50727.762', gccversion='', gccosandvers=''
    intsize=4, longsize=4, ptrsize=4, doublesize=8, byteorder=1234
    d_longlong=undef, longlongsize=8, d_longdbl=define, longdblsize=10
    ivtype='long', ivsize=4, nvtype='double', nvsize=8, Off_t='__int64',
lseeksize=8
    alignbytes=8, prototype=define
  Linker and Libraries:
    ld='link', ldflags ='-nologo -nodefaultlib -debug -opt:ref,icf
-libpath:"D:\Perl\lib\CORE"  -machine:x86'
    libpth=\lib
    libs=  oldnames.lib kernel32.lib user32.lib gdi32.lib winspool.lib
comdlg32.lib advapi32.lib shell32.lib ole32.lib oleaut32.lib
netapi32.lib uuid.lib ws2_32.lib mpr.lib winmm.lib  version.lib
odbc32.lib odbccp32.lib msvcrt.lib
    perllibs=  oldnames.lib kernel32.lib user32.lib gdi32.lib
winspool.lib  comdlg32.lib advapi32.lib shell32.lib ole32.lib
oleaut32.lib  netapi32.lib uuid.lib ws2_32.lib mpr.lib winmm.lib
version.lib odbc32.lib odbccp32.lib msvcrt.lib
    libc=msvcrt.lib, so=dll, useshrplib=true, libperl=perl510.lib
    gnulibc_version=''
  Dynamic Linking:
    dlsrc=dl_win32.xs, dlext=dll, d_dlsymun=undef, ccdlflags=' '
    cccdlflags=' ', lddlflags='-dll -nologo -nodefaultlib -debug
-opt:ref,icf  -libpath:"D:\Perl\lib\CORE"  -machine:x86'

Locally applied patches:
    ACTIVEPERL_LOCAL_PATCHES_ENTRY
    32809 Load 'loadable object' with non-default file extension
    32728 64-bit fix for Time::Local


@INC for perl 5.10.0:
    C:/Temp/Perl/site/lib
    C:/Temp/Perl/lib
    .


Environment for perl 5.10.0:
    HOME (unset)
    LANG (unset)
    LANGUAGE (unset)
    LD_LIBRARY_PATH (unset)
    LOGDIR (unset)
 
PATH=C:\WINDOWS\system32;C:\WINDOWS;C:\WINDOWS\System32\Wbem;C:\batch;C:
\Program Files\Common Files\Roxio Shared\DLLShared\;c:\Program
Files\Microsoft SQL Server\90\Tools\binn\;C:\Program
Files\Subversion\bin;C:\Program Files\Sybase\SQL Anywhere
8\win32;C:\Program Files\Sybase\Shared\win32;C:\Program Files\Sybase\SQL
Anywhere 8\drivers;C:\Program Files\Sybase\Shared\Sybase Central
4.1;C:\Program Files\Sybase\Shared\Open Server\bin;C:\Program
Files\Sybase\Shared\Open Server\dll;C:\Program
Files\QuickTime\QTSystem\;C:\Program Files\Git\cmd;C:\Temp\Perl\bin
    PERL_BADLANG (unset)
    SHELL (unset)

From l2ot9pa02@sneakemail.com

Steve Hay wrote​:

Run the following program under perl 5.10.0 on Windows XP​:

#!perl -wT
use strict;
use warnings;
BEGIN { $ENV{TMPDIR} = "$ENV{WINDIR}\\TEMP" };
use CGI;
my $tmpfile = new CGITempFile(1);
print "tmpfile='", $tmpfile->as_string(), "'\n";

This causes the error​:

Insecure dependency in sprintf while running with -T switch at (eval 2)
line 6.
[...]
"When perl is run under taint mode, printf() and sprintf() will now
reject any tainted format argument."
[...]
last if ! -f ($filename =
sprintf("${TMPDIRECTORY}${SL}CGItemp%d",$sequence++));

Obviously there are various ways around this (it's currently causing me
trouble with a Bugzilla system running on 5.10.0, and I've worked around
the problem by just commenting-out the unshift() line above so that the
tainted value is not a candidate), but I'm not sure what a good
permanent fix would be.

it seems to me that this would have to be fixed in CGITempFile because
the error is probably genuine.

How about changing the sprintf call to this (untested, but well...)​:

sprintf("\%s${SL}CGItemp%d", $TMPDIRECTORY, $sequence++)

The warning is about tainted stuff in the _format_, so this should fix
the issue.

Cheers,
Steffen

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

From @steve-m-hay

Steffen Mueller wrote​:

Steve Hay wrote​:

Run the following program under perl 5.10.0 on Windows XP​:

#!perl -wT
use strict;
use warnings;
BEGIN { $ENV{TMPDIR} = "$ENV{WINDIR}\\TEMP" };
use CGI;
my $tmpfile = new CGITempFile(1);
print "tmpfile='", $tmpfile->as_string(), "'\n";

This causes the error​:

Insecure dependency in sprintf while running with -T switch at (eval
2) line 6.

it seems to me that this would have to be fixed in CGITempFile because
the error is probably genuine.

How about changing the sprintf call to this (untested, but well...)​:

sprintf("\%s${SL}CGItemp%d", $TMPDIRECTORY, $sequence++)

Yes, that fixes it, thanks.

Lincoln, are you happy with this fix?

From @timj

On Mon, 28 Jan 2008, Steve Hay wrote​:

last if ! -f ($filename =
sprintf("${TMPDIRECTORY}${SL}CGItemp%d",$sequence++));

Obviously there are various ways around this (it's currently causing me
trouble with a Bugzilla system running on 5.10.0, and I've worked around
the problem by just commenting-out the unshift() line above so that the
tainted value is not a candidate), but I'm not sure what a good
permanent fix would be.

Why isn't it using File​::Spec->tmpdir? That problem is solved there.

--
Tim Jenness
JAC software
http​://www.jach.hawaii.edu/~timj

From @druud62

Steffen Mueller schreef​:

How about changing the sprintf call to this (untested, but well...)​:

sprintf("\%s${SL}CGItemp%d", $TMPDIRECTORY, $sequence++)

Why not work out ${SL} as well?

Even if it is currently guaranteed to never contain anything like "%d",
etc.

--
Affijn, Ruud

"Gewoon is een tijger."

From l2ot9pa02@sneakemail.com

Hi,

Dr.Ruud schrieb​:

Steffen Mueller schreef​:

How about changing the sprintf call to this (untested, but well...)​:

sprintf("\%s${SL}CGItemp%d", $TMPDIRECTORY, $sequence++)

Why not work out ${SL} as well?

Even if it is currently guaranteed to never contain anything like "%d",
etc.

No specific reason. In fact, $SL is the directory separator of the
system and thus very unlikely to ever contain a format-relevant
substring. But really, I just made the smallest possible change - it's
not my code after all. I would probably have written

$TMPDIRECTORY . $SL . sprintf('%d', $sequence++)

but that's purely a matter of preference.

Cheers,
Steffen

From @lstein

It looks fine to me. I'm adding the proposed fix to 3.33.

Lincoln

On Jan 30, 2008 4​:13 AM, Steve Hay <SteveHay@​planit.com> wrote​:

Hi Lincoln,

Not sure if you saw the email below that I copied you with the other
day, but I noticed that you just replied to another thread from a
different address, so I thought I'd forward to that address too.

Shall I go ahead and apply Steffen's fix to bleadperl, or would you
rather fix things differently?

Steve

Steve Hay wrote​:

Steffen Mueller wrote​:

Steve Hay wrote​:

Run the following program under perl 5.10.0 on Windows XP​:

#!perl -wT
use strict;
use warnings;
BEGIN { $ENV{TMPDIR} = "$ENV{WINDIR}\\TEMP" };
use CGI;
my $tmpfile = new CGITempFile(1);
print "tmpfile='", $tmpfile->as_string(), "'\n";

This causes the error​:

Insecure dependency in sprintf while running with -T switch at (eval
2) line 6.

it seems to me that this would have to be fixed in CGITempFile
because the error is probably genuine.

How about changing the sprintf call to this (untested, but well...)​:

sprintf("\%s${SL}CGItemp%d", $TMPDIRECTORY, $sequence++)

Yes, that fixes it, thanks.

Lincoln, are you happy with this fix?

--
Lincoln D. Stein
Cold Spring Harbor Laboratory
1 Bungtown Road
Cold Spring Harbor, NY 11724
(516) 367-8380 (voice)
(516) 367-8389 (fax)
FOR URGENT MESSAGES & SCHEDULING,
PLEASE CONTACT MY ASSISTANT,
SANDRA MICHELSEN, AT michelse@​cshl.edu

From @steve-m-hay

OK, thanks. Now applied to bleadperl as #33143.

________________________________

From​: Lincoln Stein [mailto​:lincoln.stein@​gmail.com]
Sent​: 30 January 2008 15​:12
To​: Steve Hay; perl5-porters@​perl.org
Subject​: Re​: FW​: [perl #50322] CGITempFile causes "Insecure dependency
in sprintf" in perl 5.10.0

It looks fine to me. I'm adding the proposed fix to 3.33.

Lincoln

On Jan 30, 2008 4​:13 AM, Steve Hay <SteveHay@​planit.com> wrote​:

  Hi Lincoln,
 
  Not sure if you saw the email below that I copied you with the
other
  day, but I noticed that you just replied to another thread from
a
  different address, so I thought I'd forward to that address too.
 
  Shall I go ahead and apply Steffen's fix to bleadperl, or would
you
  rather fix things differently?
 
  Steve
 

  Steve Hay wrote​:
  > Steffen Mueller wrote​:
  >> Steve Hay wrote​:
  >>> Run the following program under perl 5.10.0 on Windows XP​:
  >>>
  >>> #!perl -wT
  >>> use strict;
  >>> use warnings;
  >>> BEGIN { $ENV{TMPDIR} = "$ENV{WINDIR}\\TEMP" };
  >>> use CGI;
  >>> my $tmpfile = new CGITempFile(1);
  >>> print "tmpfile='", $tmpfile->as_string(), "'\n";
  >>>
  >>> This causes the error​:
  >>>
  >>> Insecure dependency in sprintf while running with -T switch
at (eval
  >>> 2) line 6.
  >>
  >> it seems to me that this would have to be fixed in
CGITempFile
  >> because the error is probably genuine.
  >>
  >> How about changing the sprintf call to this (untested, but
well...)​:
  >>
  >> sprintf("\%s${SL}CGItemp%d", $TMPDIRECTORY, $sequence++)
  >
  > Yes, that fixes it, thanks.
  >
  > Lincoln, are you happy with this fix?
 

--
Lincoln D. Stein
Cold Spring Harbor Laboratory
1 Bungtown Road
Cold Spring Harbor, NY 11724
(516) 367-8380 (voice)
(516) 367-8389 (fax)
FOR URGENT MESSAGES & SCHEDULING,
PLEASE CONTACT MY ASSISTANT,
SANDRA MICHELSEN, AT michelse@​cshl.edu

From @steve-m-hay

Now fixed in bleadperl by #33143.

@steve-m-hay - Status changed from 'open' to 'resolved'