CGI file upload file name parsing errors

[p5pRT]

Migrated from rt.perl.org#37607 (status was 'resolved')

Searchable as RT37607$

[p5pRT]

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

Perl Info

``` 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 ```

[p5pRT]

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"; }

[p5pRT]

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.

[p5pRT]

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

[p5pRT]

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

[p5pRT]

From marko.asplund@kronodoc.com

cgipm.patch

```diff --- /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/ ) ? ```

[p5pRT]

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.

[p5pRT]

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