Re: (x4) Company hacks through my Perl's Website Security hole

[jarich]

also wanted to ask you guys to explain to me the correction that took place.....

@files = <../data/texts/*>;

$file = param("select") || $files[rand(@files)];

undef @isfile{@files};
exists $isfile{$file} or die;

open(IN, "../data/texts/$file") or die $!;

What this correction does is ensure that the value in $file actually exists in ../data/texts/. If it does not exist there then your program dies.

To understand HOW it does that I suggest you read up on undef, exists and hash slices.

Your open is still broken. Read open for information on how to use three argument open. Basically you should make your opens look like this:

open (FILEHANDLE, MODE, FILENAME) or die $!;

so in this case:

open (IN, "<", "../data/texts/$file") or die $!;

Notice that we pass in the less than in a separate argument. You should do this, it makes your scripts much more secure.

tell me if it has any other flaws that i cant see since i am a newbie...

As mentioned to you many times before. Use taint mode. read perlsec and a good Perl book. There's no excuse (not even being a newbie) for not knowing how to validate your data. In this case it would be this easy:

#!/usr/bin/perl -wT

$ENV{PATH} = "";
# ... use statements and other code

my @files = <../data/texts/*>;

my $file = param("select") || $files[rand(@files)];
($file) = ($file =~ /^([\w.-]+)$/);         # Clean filename
die "Invalid filename: $file" unless $file; # Die if empty

my %isfile;
undef @isfile{@files};
exists $isfile{$file} or die "Invalid filename: $file";

open(IN, "<", "../data/texts/$file") or die $!;

Notice that we only need to make 3 changes here. The first is to add -T up on the shebang line. The second is to set our environment path. The third is the most interesting. Here we make sure that the filename only contains letters, numbers, underscores, dots and hyphens. Anything else (which would include the sequence ../) is forbidden. If the file contains things that are not in this list then we die with an error. I made a few extra changes, (such as declaring the variables and using the 3 argument version of open, but they're not strictly necessary to enable taint checking.

Using taint checking is essential for CGI scripts.

Your script would be better for using strict too. This is a great way to make your life easier. You've probably been told this 100 times already, I hope you don't feel that being a newbie means you don't have to do this either.

I hope you learn

jarich