in reply to Re: Re: Possible Security Problem
in thread Possible Security Problem

I would sanitize everything anyway. Although the directories are under your control, think about these: I'm not sure if data read from readdir() is considered tainted or not, but you should check everything. Come up with a regex that describes all the valid directory names that you expect to have, and check against that. Something like this: 
foreach (@dir) {
  s/^(\w+)$/$1/ || do { generate some error message, or 
                        skip this directory };
}

[download]
Which will allow directory names with any alphanumeric characters plus the underscore. Remember not to check for invalid characters (you could miss some), but to only allow valid ones (like the code above).

A similar untainting should probably be applied to user-supplied data, plus checking that it is in the list of valid directories.

--ZZamboni

Replies are listed 'Best First'.
(Ovid - readdir) Re(2): Possible Security Problem
by Ovid (Cardinal) on May 01, 2001 at 19:19 UTC

    ZZamboni wrote:

    I'm not sure if data read from readdir() is considered tainted or not, but you should check everything.

    Your advice about checking everything is good. I just wanted to mention that the list of directory names from readdir is will result in tainted data. Perl assumes that everything from outside of the program is tainted, even directory names that have been read in.

    Cheers,
    Ovid

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

[reply]
Re: Re: Re: Re: Possible Security Problem
by Stamp_Guy (Monk) on May 01, 2001 at 17:41 UTC
    Hi ZZamboni,
    I haven't sanitized everything yet, but here's the code I am using. Could you look it over and tell me what you think as far as security goes? 
    #!c:\perl\bin\perl.exe -wT
use strict;

use CGI qw(:standard);
my $cgi = new CGI;

my $element = $cgi->param('element');
my $type = $cgi->param('type');
my $page = $cgi->param('page');
my $dir="$page";
my @dir;
my $safedir;
opendir(TEXTFILES, "text_files") || die "Couldn't open the text file d
+irectory: $!";
@dir = grep { $_ ne "." && $_ ne ".." && -d "./text_files/$_" } readdi
+r (TEXTFILES);
closedir(TEXTFILES);
foreach (@dir) {
    if ($_ eq "$dir") {
        $safedir = "text_files/$_";
        last;
    }
}
######################################################################
+###########
# SPIT OUT THE FORM
######################################################################
+###########
if ($type eq "text") {
    opendir(CONTENTFILES, "$safedir") || die "Couldn't open the $safed
+ir directory: $!";
    my @files=grep(/\.txt$/i, readdir CONTENTFILES);
    closedir(CONTENTFILES);
    my $file_to_change;
    foreach (@files) {
        if ($_ eq "$element.txt") {
            $file_to_change = "$_";
            last;
        }
    }
    if ($file_to_change =~ /(\w+\.txt)/) {
        my $safe_file_to_change = $1;
        open(FILE, "$safedir/$safe_file_to_change") || die "Couldn't o
+pen $safe_file_to_change: $!";
        my @text_to_change=<FILE>;
        close(FILE);
        use HTMLTMPL;
        my $templ = new HTMLTMPL;
        $templ->src('text_form.html');
        my $title=ucfirst($element);
        $templ->title($title);
        $templ->element($element);
        $templ->text_to_change(@text_to_change);
        $templ->page($page);
        $templ->output('Content-Type: text/html');
    }
}

######################################################################
+###########
# CHANGE THE FILE
######################################################################
+###########
if ($type eq "text_change") {
    my $text = $cgi->param('text');
    opendir(CONTENTFILES, "$safedir") || die "Couldn't open the $safed
+ir directory: $!";
    my @files=grep(/\.txt$/i, readdir CONTENTFILES);
    closedir(CONTENTFILES);
    my $file_to_change;
    foreach (@files) {
        if ($_ eq "$element.txt") {
            $file_to_change = "$_";
            last;
        }
    }
    $file_to_change = "$safedir/$file_to_change";
    if ($file_to_change =~ /(text_files\/\w+\/\w+\.txt)/) {
        my $safe_file_to_change = $1;
        open(FILE, ">$safe_file_to_change") || die "Couldn't open $saf
+e_file_to_change: $!";
        print FILE $text;
        close(FILE);
    }
    use HTMLTMPL;
    my $templ = new HTMLTMPL;
    $templ->src('sucess_message.html');
    my $title=ucfirst($element);
    $templ->title($title);
    $templ->text($text);
    $templ->page($page);
    $templ->output('Content-Type: text/html');
}

    [download]
[reply]
[d/l]

In Section Seekers of Perl Wisdom