>Perhaps set the path of the image directory >into the script, hardcoded like so: > > $path = '/home/fubar/www/images'; > >or something like that so you are restricting to a certain directory, >and not just letting any file be read in by the cgi and sent to the
This isn't safe either. Someone could come along and request:
http://site.com/server.cgi?file=../../../../../../../etc/passwd
and get naughty files too. You want to make sure that you always sanitize the filename that you get in - never trust it. You also want to think about this /the other way/ - instead of thinking "what should I filter out?", you should think "what is it that I want to accept?". In your case, you'd probably only want alphanumeric characters only, and a single dot (to separate the file extension).
I've done stuff similar to:
my $images = '/home/fubar/www/images'; # where your files live. my $incoming = $cgi->param('file'); # the file the user wants. my $file_path = File::Spec->catfile($images, $incoming);
my ($v, $directories, $f) = File::Spec->splitpath($file_path); my @path_parts = File::Spec->splitdir($directories); push(@path_parts, $f); # check the file for naughties too. return $self->error("Hi! You've attempted directory traversal. Naughty!") if scalar File::Spec->no_upwards(@path_parts) != scalar @path_parts;
The above just checks for ".." and equivalents, however. You will probably want to check your $incoming for naughty characters too, just to be safe (ie., anything not a dot or an alphanumeric).
Yeah, what he said... Morbus is correct in the fact that you can't be too paranoid in the data you accept. I was attempting to clarify the 'whitelist' term in a simplified manner.
Reading arbitrary files from disk and spitting them out to a browser is riddled with lots of scary things to think about...
(Of course /home/fubar/www/images../../../../etc/passwd isn't the same thing as /home/fubar/www/images/../../../../etc/passwd right? ;)
Pete