Re: (Ovid) Re: 52K maximum file upload?

Replies are listed 'Best First'.
(Ovid - you have a security hole) Re(3): 52K ... by Ovid (Cardinal) on Nov 29, 2000 at 07:13 UTC
Looking through your code, I can't see any obvious reason why this would be a problem. Could it be a limitation of your Web server? Another thing to check could be to use Data::Dumper to examine the contents of the CGI object to see if everything is getting through. `use Data::Dumper; print $cgi->pre( Dumper( $cgi ) );` [download] That will get you looking at the inside of the CGI object and it's a pretty hairy thing. Once you've verified that all of your filehandles are in there, you could possibly narrow things down. It's not for the faint of heart, though. In the meantime, you might want to look at a rather significant security hole you have in your script: `$file=~m/^.(\\\|\/)(.)/; # strip the remote path and keep the filenam +e $name = $2; open(LOCAL, ">$dir/$name") or die $!; #open file` [download] See that little dot star at the end of your regex? I specify the right filename and you're toast. I could use that for reverse directory traversal and append a pipe to the end of the filename to cause it to be executed instead of opened. Got any programs on your system that you don't want a cracker to run? Another problem with it is that there is no test for failure. If it does not match, $2 may have a value from a previous match. Since you're iterating over this, it's a BAD THING. Try the following regex. It assumes that only letters, numbers, and underscores are in your filename, plus the possibility of one extension delimited by one period. `( $name ) = ( $file =~ /(\w+(?:\.\w+)?)$/ ); # Note the $ which anchor +s to the end of string` [download] As a style issue, you may want to rewrite the following: `($sec,$min,$hour,$mday,$mon,$year,$wday,$yday,$isdst) = localtime(time +);` [download] Since you are only using the minutes and seconds from this, you can rewrite it as: `my ( $sec, $min ) = (localtime( time ) )[0,1];` [download] Last note: I was really trying to avoid touting my CGI course again (too much blowing my own horn is not a good thing), but I really thing you could benefit from my lesson on security. It's free and all you can eat. Cheers, Ovid Join the Perlmonks Setiathome Group or just click on the the link and check out our stats.	[reply] [d/l] [select]

Replies are listed 'Best First'.

(Ovid - you have a security hole) Re(3): 52K ...
by Ovid (Cardinal) on Nov 29, 2000 at 07:13 UTC

Data::Dumper

use Data::Dumper;
print $cgi->pre( Dumper( $cgi ) );
[download]

In the meantime, you might want to look at a rather significant security hole you have in your script:

$file=~m/^.*(\\|\/)(.*)/; # strip the remote path and keep the filenam
+e 
$name = $2; 

open(LOCAL, ">$dir/$name") or die $!; #open file
[download]

executed instead of opened

Another problem with it is that there is no test for failure. If it does not match, $2 may have a value from a previous match. Since you're iterating over this, it's a BAD THING. Try the following regex. It assumes that only letters, numbers, and underscores are in your filename, plus the possibility of one extension delimited by one period.

( $name ) = ( $file =~ /(\w+(?:\.\w+)?)$/ ); # Note the $ which anchor
+s to the end of string
[download]

As a style issue, you may want to rewrite the following:

($sec,$min,$hour,$mday,$mon,$year,$wday,$yday,$isdst) = localtime(time
+);
[download]

my ( $sec, $min ) = (localtime( time ) )[0,1];
[download]

my lesson on security

Cheers,
Ovid

Join the Perlmonks Setiathome Group or just click on the the link and check out our stats.

[reply]
[d/l]
[select]