From aspa@merlot.kronodoc.fi

Created by aspa@merlot.kronodoc.fi

I'm using the CGI module to parse HTTP POST file upload requests.
I noticed that if the file name is quoted and contains a semicolon CGI fails
to parse the name correctly. For example using 'foo;bar.txt' as the file name
would result in the following Content-Disposition line in the HTTP request​:

Content-Disposition​: form-data; name="filename1"; filename="foo;bar.txt"

which would cause CGI to fail the file name parsing.

According to RFC 1867, 2183 and 2045 the file name field value can contain
semicolons when the name is quoted.

A related issue is that when the file name parsing fails the file content
is loaded into the parsed CGI object i.e. in main memory.

I would propose the following patch to the CGI module to fix these issues​:

3258c3258,3260
< my($filename) = $header{'Content-Disposition'}=~/ filename="([^;]*)"/;
---

  \# RFC 1867\, 2183\, 2045
  my \($filename\) = $header\{'Content\-Disposition'\}=~/ filename=\(\("\[^"\]\*"\)|\(

[a-z\d!#'\*\+,\.^_\`\{\}\|\~]*))/i;

    $filename =~ s/^"\(\[^"\]\*\)"$/$1/;

3262a3265,3269

  \# prevent file content from being loaded into memory should
  \# content\-disposition parsing fail\.
  if\($header\{'Content\-Disposition'\}=~/ filename=/ && \!$filename\) \{
      $filename = "noname\.bin";
  \}

--
  aspa

Flags:
    category=library
    severity=medium

Site configuration information for perl v5.8.7:

Configured by aspa at Fri Nov  4 10:58:39 EET 2005.

Summary of my perl5 (revision 5 version 8 subversion 7) configuration:
  Platform:
    osname=linux, osvers=2.4.21-32.elsmp, archname=i686-linux
    uname='linux merlot.kronodoc.fi 2.4.21-32.elsmp #1 smp fri apr 15 21:17:59 edt 2005 i686 i686 i386 gnulinux '
    config_args='-de -Dprefix=/home/aspa/tmp/perl-5.8.7'
    hint=recommended, useposix=true, d_sigaction=define
    usethreads=undef use5005threads=undef useithreads=undef usemultiplicity=undef
    useperlio=define d_sfio=undef uselargefiles=define usesocks=undef
    use64bitint=undef use64bitall=undef uselongdouble=undef
    usemymalloc=n, bincompat5005=undef
  Compiler:
    cc='cc', ccflags ='-fno-strict-aliasing -pipe -I/usr/local/include -D_LARGEFILE_SOURCE -D_FILE_OFFSET_BITS=64 -I/usr/include/gdbm',
    optimize='-O2',
    cppflags='-fno-strict-aliasing -pipe -I/usr/local/include -I/usr/include/gdbm'
    ccversion='', gccversion='3.2.3 20030502 (Red Hat Linux 3.2.3-53)', gccosandvers=''
    intsize=4, longsize=4, ptrsize=4, doublesize=8, byteorder=1234
    d_longlong=define, longlongsize=8, d_longdbl=define, longdblsize=12
    ivtype='long', ivsize=4, nvtype='double', nvsize=8, Off_t='off_t', lseeksize=8
    alignbytes=4, prototype=define
  Linker and Libraries:
    ld='cc', ldflags =' -L/usr/local/lib'
    libpth=/usr/local/lib /lib /usr/lib
    libs=-lnsl -lgdbm -ldb -ldl -lm -lcrypt -lutil -lc
    perllibs=-lnsl -ldl -lm -lcrypt -lutil -lc
    libc=/lib/libc-2.3.2.so, so=so, useshrplib=false, libperl=libperl.a
    gnulibc_version='2.3.2'
  Dynamic Linking:
    dlsrc=dl_dlopen.xs, dlext=so, d_dlsymun=undef, ccdlflags='-Wl,-E'
    cccdlflags='-fpic', lddlflags='-shared -L/usr/local/lib'

Locally applied patches:
    


@INC for perl v5.8.7:
    /home/aspa/tmp/perl-5.8.7/lib/5.8.7/i686-linux
    /home/aspa/tmp/perl-5.8.7/lib/5.8.7
    /home/aspa/tmp/perl-5.8.7/lib/site_perl/5.8.7/i686-linux
    /home/aspa/tmp/perl-5.8.7/lib/site_perl/5.8.7
    /home/aspa/tmp/perl-5.8.7/lib/site_perl
    .


Environment for perl v5.8.7:
    HOME=/home/aspa
    LANG=en_US.iso885915
    LANGUAGE (unset)
    LANGVAR=en_US.iso885915
    LD_LIBRARY_PATH (unset)
    LOGDIR (unset)
    PATH=/usr/kerberos/bin:/usr/local/bin:/bin:/usr/bin:/usr/X11R6/bin
    PERL_BADLANG (unset)
    SHELL=/bin/tcsh

From marko.asplund@kronodoc.com

The following part in the patch breaks HTML forms with multiple file upload elements when
there's one or more empty elements posted to the server​:

if($header{'Content-Disposition'}=~/ filename=/ && !$filename) {
  $filename = "noname.bin";
}

From @smpeters

[aspa - Fri Nov 04 04​:33​:48 2005]​:

The following part in the patch breaks HTML forms with multiple file
upload elements when
there's one or more empty elements posted to the server​:

if($header{'Content-Disposition'}=~/ filename=/ && !$filename) {
$filename = "noname.bin";
}

Could you please resend the patch as a diff -u (if your diff supports
it) or as a diff -c. It makes it much easier to see the changes.

Thanks.

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

From marko.asplund@kronodoc.com

[stmpeters - Tue Dec 27 05​:48​:06 2005]​:

Could you please resend the patch as a diff -u (if your diff supports
it) or as a diff -c. It makes it much easier to see the changes.

here you go.

br. aspa

From marko.asplund@kronodoc.com

--- /opt/kronodoc/perl/ARCHIVE/kb3401dr2/lib/5.8.7/CGI.pm	2005-07-11 10:15:47.000000000 +0300
+++ CGI.pm	2005-11-04 15:03:42.000000000 +0200
@@ -19,7 +19,7 @@
 #   http://stein.cshl.org/WWW/software/CGI/
 
 $CGI::revision = '$Id: CGI.pm,v 1.181 2005/05/13 21:45:26 lstein Exp $';
-$CGI::VERSION='3.10';
+$CGI::VERSION='3.10-kd1';
 
 # HARD-CODED LOCATION FOR FILE UPLOAD TEMPORARY FILES.
 # UNCOMMENT THIS ONLY IF YOU KNOW WHAT YOU'RE DOING.
@@ -3255,7 +3255,11 @@
         $param .= $TAINTED;
 
 	# Bug:  Netscape doesn't escape quotation marks in file names!!!
-	my($filename) = $header{'Content-Disposition'}=~/ filename="([^;]*)"/;
+	# See RFC 1867, 2183, 2045
+	# NB: File content will be loaded into memory should
+	# content-disposition parsing fail.
+	my ($filename) = $header{'Content-Disposition'}=~/ filename=(("[^"]*")|([a-z\d!\#'\*\+,\.^_\`\{\}\|\~]*))/i;
+        $filename =~ s/^"([^"]*)"$/$1/;
 	# Test for Opera's multiple upload feature
 	my($multipart) = ( defined( $header{'Content-Type'} ) &&
 		$header{'Content-Type'} =~ /multipart\/mixed/ ) ?

From @smpeters

On Tue Dec 27 23​:40​:27 2005, aspa wrote​:

[stmpeters - Tue Dec 27 05​:48​:06 2005]​:

Could you please resend the patch as a diff -u (if your diff supports
it) or as a diff -c. It makes it much easier to see the changes.

here you go.

br. aspa

Sorry, but this response seems to never have made it to any mailing list. I've just applied this
patch as change #32683.

@smpeters - Status changed from 'open' to 'resolved'