Security with open() in CGI scripts

[Coplan]

Once again, I have another CGI question, but this time relative to security. I'll admit, I know very little about security, and while the -T flag does help, I know that there is always something I might overlook. Just last night, my friend noticed in one of his own scripts that there was a bit of a security loophole in the open command. Apparently, someone figured out how to use some sort of escape commands in the URL to hack into his system. His script's purpose was different -- basically a way for people to view static HTML documents which were parsed into his web-page's table structure. Mine, on the other hand, is a bit different, but I imagine I could potentially have the same loop hole.

Before I show you my little sub-routine, a little background. For obvious reasons, I am not going to display the whole program here. The premise of this sub-routine is that I have a template file, an HTML document, that has commands within it (for example: <!-- fillin(getContent) --> ). Basically, a template is loaded, and based on the input into the CGI, the content will replace the fillin() tags. That in itself is not a security hole as one might think, because I have it set to check whether said commands actually exist earlier in the script (an earlier subroutine). Anyhow, my concern is with the open() command. Here's my code:

# getHTML: Get template and fill in with generated data
sub getHTML {
  $UserInfo = getUserInfo($tempUID);
  $theme = getTheme($UserInfo->{Theme});

  my $template = $theme->{ThemeDir}."index.html";

  my $HTML .= <<END;
Pragma: no-cache
Cache-Control: no-cache,must-revalidate,max-age=0,no-store,private
Expires: -1
Content-type: text/html

END

  open(FILE, "<$template") or die "Template : Couldn't open $template 
+: $!\n";
    while (<FILE>) { $HTML .= $_ }
  close(FILE);

  while ($HTML =~ /<!-- fillin\((.+?)\) -->/) {
    my $command_output = &$1($call);
    $HTML =~ s/<!-- fillin\(.+?\) -->/$command_output/;
  }

  return $HTML;
} # end getHTML()

Is there any sort of protection I should be adding over the opening of such a file? Maybe some sort of regexp to disallow anything that shouldn't belong? I know in my friend's case, his program allowed user input through the URL. In my case, there is no way for the person to input that template file. The template file is based on a theme that they can select from the database. The file itself is stored within the database.

While I am at it, is there a safer way, in general, to open files in perl? Most of what I do with perl is web development, so this might be useful to know anyhow.

Once again, thank you for your help. I've learned a lot from this wonderful community, and I hope to continue doing so.

--Coplan

[rob_au]

A quick cursory look of your code doesn't reveal much ... It doesn't look as if you are running under strict as there doesn't appear to be any definition of scope of some of your variables. Nevertheless, the snippet you have given doesn't show enough for any judgement to be made of its 'fitness' for any given task - There is no indication of where you are deriving some of your values from, in particular, $tempUID and $call, without which any assessment of security tightness of your code would be flawed.

A general pragma to remember with regard to CGI security is to never trust anything which comes from the browser - Irrelevant of whether it be query arguments, cookie data or user submitted information, don't trust it! This is vitally important where any of the submitted information may be used to manipulate the filesystem or process tree directly - In such instances, you should be excluding everything and then selectively permitting that which is vetted and permissable. This can lead to a great deal of code overhead but given the consequences of a failure in security, this is a little price to pay in development time.

With regard to open and security inherit to the command itself, this comes down very much to the arguments which are passed to it - There has previously been the discussion on the 2-argument invocation of open with specific reference to passed arguments. But at the end of the day, the security concerns will center on how the data is parsed and vetted prior to being passed to open - Has the data been checked for shell escape characters? Does the target file to be opened exist? Is the target file a directory or symbolic link? Are the permission and ownership rights of the target file as expected and allowed? The list goes on ...

In short, limit the allowable parameters, code defensively and don't trust anything sent from the browser.

 

perl -e 's&&[e-mail]&&&split/[@.]/&&s&.com.&_&&&print'

[Trimbach]

...and as an add on to rob_au's excellent post, here's an idea to increase security, especially for open calls and such. Instead of passing directory names and filenames directly from the browser (which means worrying about shell escape characters and such) why not pass tokens that correspond to directories and files that are hard-coded into your script. So instead of

my $template = $theme->{ThemeDir}."index.html";

which makes you worry about $theme->{ThemeDir} why not

my %themes = {1 => '/dir1/', 
              2 => '/dir2/',
              3 => '/dir3/'};

my $theme = param(theme);
my $filename = "$themes{$theme}.index.html";
open (FILE, $filename);

Now everything is under control... if $theme ever contains anything it isn't supposed to then the worst thing that can happen is the hash lookup fails and the open fails. No way malicious code can hurt you, all because you insulated your open by using a hash.

Of course, this isn't always practical, but most of the time it's a good idea. This won't work if you let users name their own files, and upload (or create) those files directly on the server, but if you're doing that you're gonna have to jump through more hoops than we've covered here in order to maintain security.

Gary Blackburn
Trained Killer

[brianarn]

Another thought on the token idea here...

I'd recommend having another file external to the script, which contains some quick perl code to create the token list, such as

my %tokens = {1 => '/dir1/',
              2 => '/dir2/',
              3 => '/dir3/};

<blatently stolen from Gary's reply ;)>

and then just require this file when you need to reference the token. This way, it's easy to add to the themes without having to find where it's at in some script. I'm personally all about the use of extra files for data like this - it helps keep script sizes smaller and helps to separate the code from the data.

~Brian

[metadoktor]

In general, do not accept any input from the client (compare whatever you receive against an internal list of what is acceptable), but if you have to accept input from the client then make sure that it conforms to the kind of data that you are expecting...so make sure it doesn't exceed a certain length, that it doesn't have strange characters, that it has the proper order of chars, etc).

Some classic documents about computer security:

W3 Security FAQ by Lincoln Stein and John Stewart

Secure Programming Checklist by Simson Garfinkel and Gene Spafford

Perl CGI Problems in Phrack Magazine, Vol 9, Issue 55 by Rain Forest Puppy

metadoktor

"The doktor is in."

[smitz]

Something weird you should look out for that I came across late one night:

# parse $user_input
$database="$user_input.db";
open(FILE "<$database");

Ok, whats the problem? CGI passes user_input=rfp, and the script tries to open rfp.db. (Lets ignore the ../../ stuff for now). Then it got interesting when I passed 'user_input=rfp%00'. Perl made $database="rfp\0.db", and then tried to open $database. The results?
It opened "rfp" (or would have, had it existed). What happened to the ".db"? As you probably know, Perl allows NULL characters in its variables as data. Unlike in C, NULL is not a string delimiter. But, the underlying system/kernel calls are programmed in C, which DOES recognize NULL as a delimiter. So the end result? Perl passes "rfp\0.db", but the underlying libs stop processing when they hit the first (our) NULL.
Im sure the brainpower of monks-combined (TM) could think of some nasty applications of this feature.


Update: I had intended to credit the author, RainForestPuppy, phrack mag 55-7. Sorry for the slip-up.

Tata

SMiTZ

Added update per user request - dvergin 2002-02-26

[higle]

Another great resource that covers CGI/Web programming security basics is Ovid's online CGI programming course, particularly lesson #3, Basic Security with CGI.pm. It has great information about untainting your data, including that naughty NULL byte.

I learned tons from reading this quick and condensed course.

  higle