revdiablo has asked for the wisdom of the Perl Monks concerning the following question:

I am taking in filenames from "the evil user", and would like to know the most effective way to escape them. Basically, I want to avoid accessing files in lower directories using ../ or similar. I am not using the filenames to execute any code via the shell, so I think most metacharacters are OK. My code does the following:

my $filename = take_user_input();
$filename =~ s/\.\./\\\.\\\./g; # this is the part in question

open IN, "<", $filename or die "Cannot open file '$filename' for input: $!\n";
print join "", <IN>;
close IN;

Do you think this simple regular expression is sufficient, or are there other things I should keep in mind? In case you're wondering: yes this is for a CGI, but I'm not so sure it matters. Nothing further is done with this filename, other than opening it and printing the contents to the browser.

  • Comment on escaping filenames taken in via user input

Replies are listed 'Best First'.
Re: escaping filenames taken in via user input
by Aristotle (Chancellor) on Oct 30, 2002 at 00:27 UTC

    Sure, you think so. Do you know?

    What if I add a ; in the command and make the shell execute another command than you intended to as well?

    You shouldn't even try to escape - check whether there's any character in your input that you didn't explicitly allow and if so, reject that input. The only way to achieve security is with a "default deny" policy that only explicitly allows known good input, rather than "default accept" that tries to reject known bad input. Someone may come up with a new way to pass bad input that slips through your filters, and bam.

    You should if at all possible avoid the shell, too.

    Maybe if you explain what you're really trying to do, someone could point you in a better direction.

    Makeshifts last the longest.

[reply]

      I do avoid the shell, as I stated. The only thing I do with the filename is pass it to Perl's builtin open.

      As for what this CGI does; it takes a filename in and prints the contents of it to the browser. That's all. Literally. :) I eventually plan to add logging and possibly access control. I realize web servers already handle all these tasks exceptionally well, but the problem is I don't have access to the web server logs or configuration. This CGI is a workaround for the restricted system I'm using to host my site.

[reply]
[d/l]
        Right. Then I'd say reject any filename that contains a dot - simple as that. Also, use the three-argument form of open as in open FH, "<", $filename; to avoid having tricks played on you with the magic open features of the two-argument forum. See Ovid's excellent CGI course for more information on the topic of security in CGI scripts.

        Makeshifts last the longest.

[reply]
        the problem is I don't have access to the web server logs or configuration

        so it seems that your "evil user" has more access than you do. You are only escaping .. s/\.\./\\\.\\\./g; so if I supply /etc/passwd as my input, how does your script handle it?

        You probably should split the path into its components, and then decide from there what the file is.

        He who asks will be a fool for five minutes, but he who doesn't ask will remain a fool for life.

        Chady | http://chady.net/
[reply]
[d/l]
[select]
Re: escaping filenames taken in via user input
by gjb (Vicar) on Oct 30, 2002 at 00:37 UTC

    I guess a combination of File::Basename and IO::File should help. You can use File::Basename to properly retrieve path and filename, while IO::File should guarantee that no funny stuff happens while opening the file.

    Anyway, there be dragons...

    Hope this helps, -gjb-

[reply]
Re: escaping filenames taken in via user input
by stupidius (Initiate) on Oct 30, 2002 at 00:48 UTC
    This is what I would do:

    my $filename = take_user_input();

$filename =~ s/\W//g; # allows only alphanumerics and the  underscore

$filename = $filename . ".txt"; # forces them to only open .txt files

if (-T $filename) # checks to see if the file is a text file
{
  open IN, "<", $filename or die &cant_open_file;
  print join "", <IN>;
  close IN;
}

else
{
  &cant_open_file;
}

    [download]
[reply]
[d/l]
      Minor nitpick for efficiency: don't do join '', <FH> - ever. Since you're only going to glue all the lines together anyway, don't ask for the file to be delivered line by line in the first place. Either localize $/ or use read. I'd do the first in this case. 
      if(..) {
    open my $fh, "<", $filename or die &cant_open_file;
    local $/ = \8192; # read in 8 kbyte chunks rather than line by lin
+e
    print while <$fh>;
}

      [download]
      See perldoc perlvar on the intricacies of $/. Note that this code uses a lexical to store the filehandle - it needs Perl 5.6 or newer to work that way, but affords us the luxury of not closing the file explicity. It just gets autoclosed when the $fh goes out of scope at the end of the block.

      Makeshifts last the longest.

[reply]
[d/l]
        You would be correct. I didnt even notice the join. I just pasted that part <blushes>. Just focused on using the \W on the filename. Good point!
[reply]

      Escaping or removing all dots was something I originally considered, but decided against, because it would mangle my file exension. I didn't think to append the extension automatically... duh! How simple. :)

      Also, Aristotle++ for pointing out the problem with join "", <FH>. This is something I use often, for no reason other than it seems clear and obvious to me... but if it's grossly inefficient, I'll gladly switch to something else.

[reply]
[d/l]
        Well, not necesarily grossly inefficient - but it's just useless work. I don't generally microoptimize, but there's absolutely no gain to be had by the less efficient form, since typing do { local $/; <FH> } instead takes no more programmer time and doesn't make maintenance any harder. So it's a case of simple economics. </rambling type="short"> :-)

        Makeshifts last the longest.

[reply]
[d/l]

Back to Seekers of Perl Wisdom