Re: open file using variable passed by form

I have a number of comments:

You should Use strict and warnings (also, since you're currently using -w, see What's wrong with -w and $^W).
For debugging, you can run CGI.pm scripts from the command line, e.g. perl -T comment.cgi 'event=foo&newCOMMENT=bar' and you can inspect the contents of variables (such as the filename, if you were to store it in a variable) with modules like Data::Dumper - see also the Basic debugging checklist.
When you are getting 500 server errors from CGI scripts, you should check the server's error logs for the exact error message. See also CGI Help Guide and Troubleshooting Perl CGI scripts (I've linked you to these before). There is also CGI::Carp's use CGI::Carp qw/fatalsToBrowser/;, although you should not use this in a production environment as it may reveal too many details of the server to an attacker.
You should use the three-argument form of open, lexical filehandles, and always check the return value for errors, for example: open my $fh, '>>', $filename or die "$filename: $!";
Since you can't always be sure what environment CGI scripts will be run in, you should use absolute pathnames.
But the biggest issue I see with this script is security. You are taking form input, completely unchecked, and using it directly in a filename. An attacker could inject arbitrary values here. At least you are using Taint mode (-T) - which is currently where your errors are coming from (and I think it's a good thing you're getting errors in this case...). But even so I would be very strict with the values that are allowed for those variables. (Also, what if $newCOMMENT contains newlines?)
Using flat files is not necessarily the most efficient or safe way to do this. I see several race conditions here, such as $commentUID (what if there are multiple comments submitted within the same second?), opening two files separately (what if two processes are running concurrently? you may get out-of-order lines), and also no file locking going on. This would be a good place to use a database instead.

Now, having said all that, it's still possible to do something like what you want. Although as I said I wouldn't recommend using flat files, this is just for the sake of showing one way to do it. Note that I am only using a single file - but a warning about this - I'm using flock, which is not supported on all filesystems!

#!/usr/bin/perl -T
use warnings;
use strict;
use CGI qw/param header/;
use Fcntl qw/:flock :seek/;

my $COMMENTFILE = '/path/to/comments.txt'; # absolute pathname

if ( length param('newCOMMENT') ) {
    # untaint form input with strict regexes
    my ($event) = param('event') =~ /\A(\w+)\z/
        or die "bad event";
    my ($comment) = param('newCOMMENT') =~ /\A([\w\h]+)\z/
        or die "bad comment";
    # we use "---" as a marker below, so strip that out
    $comment=~s/^---//gm;
    
    open my $fh, '>>', $COMMENTFILE or die "$COMMENTFILE: $!";
    # lock the file
    flock($fh, LOCK_EX) or die "flock $COMMENTFILE: $!";
    # in case someone else has written to the file in the meantime
    seek($fh, 0, SEEK_END) or die "seek $COMMENTFILE: $!";
    # I've just picked a format for the flat file
    print $fh "--- ",time," ",$event,"\n";
    # we know $comment doesn't contain "---" because of the regex
    print $fh $comment,"\n";
    
    flock($fh, LOCK_UN) or die "un-flock $COMMENTFILE: $!";
    close $fh;
}

print header('text/plain');
print "Hello, World\n";
[download]

Comment on Re: open file using variable passed by form Select or Download Code

Replies are listed 'Best First'.
Re^2: open file using variable passed by form by michael.kitchen (Novice) on Mar 19, 2018 at 04:52 UTC
You were right! My problem (part or all, not sure) was -T. I have something that is working (with -T) and it came from a combination of your code and poj's. I couldn't quite follow all of your code and was wondering if you might take a little bit more time and explain a couple of things for me. I truly like to understand what is going on, at least to some small degree. :) `if ( length param('newCOMMENT') ) # deciding if a new comment exists? # untaint form input with strict regexes - don't need to know exactly +what is happening my ($event) = param('event') =~ /\A(\w+)\z/ or die "bad event"; # just wondering why code is different for 'newComment' my ($comment) = param('newCOMMENT') =~ /\A([\w\h]+)\z/ or die "bad comment";` [download] Again,thanks for taking your time to originally post and for any time spent replying to this one. And thanks for giving me a lot more to think about. FYI though, I now have plans for a better UID, plan on locking files while in use, and changing from textarea to standard text input box.	[reply] [d/l]
Re^3: open file using variable passed by form by haukex (Archbishop) on Mar 19, 2018 at 07:59 UTC
`length param('newCOMMENT')` This avoids warnings when `param` returns `undef` (at least on Perl 5.12 and up). As for the regexes, have a look at perlrequick and perlretut. Using them, I make sure that `param('event')` contains only "word" characters (which excludes things like dots, slashes, or backslashes), and I make sure that `newCOMMENT` includes only "word" characters and horizontal whitespace (which excludes, for example, newlines; see perlrebackslash and perlrecharclass for all the details). I then use capture groups to "untaint" the values (see perlsec). You may find that the regexes are too restrictive, in which case you can add allowed characters, but be very careful with this - adding too many or the wrong ones will open the security holes again. This is another good reason to not use form input for filenames and use a database instead, where these things are not an issue if you use the right tools - see Bobby Tables.	[reply] [d/l] [select]


"be consistent"
	PerlMonks