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

I am a new programmer and I stopped quite a while ago because people seemed to be overly rude and bashed me for my errors even though I'm new...

I was wondering if anyone could give me any useful feedback on why the following script I wrote is insecure and help me make it better. Don't talk all high and complex trying to make you look smart and me look dumb, remember, I don't know much about this language yet and I want to make sure I do things right from the beginning.

Any help would me much appreciated, thanks.


#!/usr/local/bin/perl -w use strict; # Let's have some fun my'ing things, shall me? my %form; my $sendmail; my $webmaster; my $yourname; my $thanks; use CGI; my $query = CGI->new; print $query->header; %form = %{$query->Vars}; $sendmail = "/var/qmail/bin/qmail-inject"; $webmaster = 'sulfericacid@qwest.net'; ( $thanks = <<'END_OF_THANKS' ) =~ s/^\s+//gm ; Thank you for visiting my website! Things are always changing so I hope you pop back again! If you asked any questions, please allow upto 24 hours for a response. END_OF_THANKS $yourname = 'Aaron'; # $signame = 'Aaron Anderson\n'; # $sigurl = 'www.yourdowmain.com\n"; ### Make sure things are being completed, die slackers! if ($webmaster eq "") { print "Webmaster, please include your email address!\n"; } if ($form{'usermail'} eq "") { print "Ok, buddy, how do you expect me to email you back if you FORGET + to leave yoru email address?!?\n"; } if ($form{'username'} eq "") { print "Please click back and type in your name so I can spy on you!\n" +; } if ($form{'message'} eq "") { print "Only a severe slacker would try to submit a form without leavin +g a message!\n"; } # Mail to Webmaster open (MAIL, '|-', "$sendmail -t") or die $!; print MAIL "To: $webmaster\n"; print MAIL "From: $form{'usermail'}\n"; print MAIL "Subject: Insert subject here!\n\n"; print MAIL "Name- $form{'username'}\n"; print MAIL "Url- $form{'userweb'}\n"; print MAIL "Message- $form{'message'}\n"; print MAIL "Ip- $ENV{'REMOTE_ADDR'}\n"; close (MAIL); # Mail to User open (MAIL, '|-', "$sendmail -t") or die $!; print MAIL "To: $form{'usermail'}\n"; print MAIL "From: $form{'weburl'}\n"; print MAIL "Subject: Thank you for signing my form!\n\n"; print MAIL "$thanks\n"; print MAIL "You said:\n"; print MAIL "$form{'message'}\n"; close (MAIL); # Let's give them something to look at, might as well, they were nice +enough to fill out the form! print <<end_of_results; <html> <head> <title>Results Page</title> </head> <table width="75%" border="0"> <tr> <td colspan="2"> <p>Thank you for filling out the form. If you are awaiting assis +tance and you asked a question, please allow upto 24 hours for my to get + back to you.</p> <p>- $yourname</p> </td> </tr> <tr> <td colspan="2"><font color="#9933CC">Here is a copy of what you s +ent...</font></td> </tr> <tr> <td width="9%"><font color="#666666">name:</font></td> <td width="91%"><font color="#999999">$form{'username'}</font></td +> </tr> <tr> <td width="9%"><font color="#666666">email:</font></td> <td width="91%"><font color="#999999">$form{'usermail'}</font></td +> </tr> <tr> <td width="9%"><font color="#666666">url:</font></td> <td width="91%"><font color="#999999"><a href="$form{'weburl'}">$f +orm{'weburl'}</a></font></td> </tr> <tr> <td width="9%"><font color="#666666">message:</font></td> <td width="91%"><font color="#999999">$form{'message'}</font></td> </tr> <tr> <td width="9%"><font color="#666666">Rec. IP:</font></td> <td width="91%"><font color="#999999">$ENV{'REMOTE_ADDR'}</font></ +td> </tr> <tr> <td width="9%">&nbsp;</td> <td width="91%">&nbsp;</td> </tr> </table> end_of_results

Replies are listed 'Best First'.
Re: Insecurities in my scripting
by grep (Monsignor) on Nov 20, 2002 at 05:20 UTC

    Just a quick couple of notes

    use CGI;
    This is great you're already ahead of most new CGI programmers

    print MAIL "To: $form{'usermail'}\n";
    this right here, let's you become a spam portal. You have just allowed anyone who can find your page send email to anyone including UCE. You should decide if this user email is really necessary. If it is necessary preset a message; don't take a message from a form value, this will at least make it useless for spammers.

    Use taint checking, to validate and clean your form data. Never trust form data.

    Check out Ovid's excellent "Web Programming Using Perl" course.



    grep
    Mynd you, mønk bites Kan be pretti nasti...
Re: Insecurities in my scripting
by fireartist (Chaplain) on Nov 20, 2002 at 09:12 UTC
    I'd just like to expand a little on grep's comments above, to explain why this script could be used as a spam portal.

    print MAIL "To: $form{'usermail'}\n";
    Somebody could simply copy and paste an entire mailing list into the 'usermail' field and submit it.
    The email would then be sent to everyone in that list.
    victim1@domain.com, victim2@domain.com, victim3@domain.com, etc...
    You get the picture.
    This is made worse because the $form{'message'} is sent as the body of the email, meaning the (ab)user could enter their own adverts, urls etc into the 'message' field, so everyone would receive spam (possibly offensive) and it would appear to have been sent by you.

    If you switch on taint mode

    #!/usr/bin/perl -wT
    Then perl will force you to run every user inputted data through a regex before you can send it outside of your program, such as to sendmail.
    my $username; If ($form{username} =~ /^([\w.-]+)$/) { $username = $1; } else { die("invalid username"); }
    See the regex docs for help.
    You would need to run every user input through a regex. The difficult one is validating the email address, have a search around the monastery for help with that.

      Are you virtually saying all I need to do is edit the first line, as i did below, for it to be more secure?

      #!/usr/local/bin/perl -wT use strict; # Let's have some fun my'ing things, shall me? my %form; my $sendmail; my $webmaster; my $yourname; my $thanks; use CGI; my $query = CGI->new; print $query->header; %form = %{$query->Vars}; $sendmail = "/var/qmail/bin/qmail-inject"; $webmaster = 'sulfericacid@qwest.net'; ( $thanks = <<'END_OF_THANKS' ) =~ s/^\s+//gm ; Thank you for visiting my website! Things are always changing so I hope you pop back again! If you asked any questions, please allow upto 24 hours for a response. END_OF_THANKS $yourname = 'Aaron'; # $signame = 'Aaron Anderson\n'; # $sigurl = 'www.yourdowmain.com\n"; ### Make sure things are being completed, die slackers! if ($webmaster eq "") { print "Webmaster, please include your email address!\n"; } if ($form{'usermail'} eq "") { print "Ok, buddy, how do you expect me to email you back if you FORGET + to leave yoru email address?!?\n"; } if ($form{'username'} eq "") { print "Please click back and type in your name so I can spy on you!\n" +; } if ($form{'message'} eq "") { print "Only a severe slacker would try to submit a form without leavin +g a message!\n"; } # Mail to Webmaster open (MAIL, '|-', "$sendmail -t") or die $!; print MAIL "To: $webmaster\n"; print MAIL "From: $form{'usermail'}\n"; print MAIL "Subject: Insert subject here!\n\n"; print MAIL "Name- $form{'username'}\n"; print MAIL "Url- $form{'userweb'}\n"; print MAIL "Message- $form{'message'}\n"; print MAIL "Ip- $ENV{'REMOTE_ADDR'}\n"; close (MAIL); # Mail to User open (MAIL, '|-', "$sendmail -t") or die $!; print MAIL "To: $form{'usermail'}\n"; print MAIL "From: $form{'weburl'}\n"; print MAIL "Subject: Thank you for signing my form!\n\n"; print MAIL "$thanks\n"; print MAIL "You said:\n"; print MAIL "$form{'message'}\n"; close (MAIL); # Let's give them something to look at, might as well, they were nice +enough to fill out the form! print <<end_of_results; <html> <head> <title>Results Page</title> </head> <table width="75%" border="0"> <tr> <td colspan="2"> <p>Thank you for filling out the form. If you are awaiting assis +tance and you asked a question, please allow upto 24 hours for my to get + back to you.</p> <p>- $yourname</p> </td> </tr> <tr> <td colspan="2"><font color="#9933CC">Here is a copy of what you s +ent...</font></td> </tr> <tr> <td width="9%"><font color="#666666">name:</font></td> <td width="91%"><font color="#999999">$form{'username'}</font></td +> </tr> <tr> <td width="9%"><font color="#666666">email:</font></td> <td width="91%"><font color="#999999">$form{'usermail'}</font></td +> </tr> <tr> <td width="9%"><font color="#666666">url:</font></td> <td width="91%"><font color="#999999"><a href="$form{'weburl'}">$f +orm{'weburl'}</a></font></td> </tr> <tr> <td width="9%"><font color="#666666">message:</font></td> <td width="91%"><font color="#999999">$form{'message'}</font></td> </tr> <tr> <td width="9%"><font color="#666666">Rec. IP:</font></td> <td width="91%"><font color="#999999">$ENV{'REMOTE_ADDR'}</font></ +td> </tr> <tr> <td width="9%">&nbsp;</td> <td width="91%">&nbsp;</td> </tr> </table> end_of_results

        If you set taint mode, your program will not be more secure, but Perl will warn you of (some) possible security problems. That is, Perl will refuse to run it if it thinks it is not secure enough.

        -- 
                dakkar - Mobilis in mobile
        
        Short answer: yes !

        However, once Taint mode is on, every piece of user entered data must be untainted, or your script will die if you try to pass that data onto an external program.

        This can only be done by assigning the value from $1 after successfully matching a regex.

        You could though, match anything with a regex such as ...
        warning: bad code

        if ( $form{usermail} =~ /(.*)/ ) { $form{usermail} = $1; }
        Taint mode will not complain, and you will be still passing insecure data to the sendmail program because that regex matched enything and everything.

        So, long answer: yes, but it's up to you to responsibly and thoughtfully untaint the data.

        As before, I recommend you read the regex tutorial to learn such things as assigning a match to $1.
        See this offsite Taint tutorial http://gunther.web66.com/FAQS/taintmode.html.

      Even worse, an attacker could put the entire messages and his own headers in the useremail parameter. Newlines are allowed in form parameters. The attacker could forge his own email headers and push the real message to the bottom.
Re: (nrd) Insecurities in my scripting
by newrisedesigns (Curate) on Nov 20, 2002 at 05:34 UTC

    It's very good to see you using CGI.pm and strict with warnings. That's a good start for a beginner programmer. ++ for that.

    Your coding style is good, however, why did you roll your own form-mailer? For security's sake, you should use something tried and true, like the fine scripts at nms. They worked out any potential problems you might come across.

    For some information about security, check out Chapter 8: Security in the Mouse book, CGI Programming with Perl.

    I'm no professional like merlyn, chromatic or any of the other fine monks here, but I can safely say that you seem to have a strong grasp of the basics of programming. It's a good start, so keep at it.

    John J Reiser
    newrisedesigns.com

Re: Insecurities in my scripting
by chromatic (Archbishop) on Nov 20, 2002 at 18:11 UTC

    Since newrisedesigns called me a professional, I feel compelled to respond. :)

    Without the -T flag, there's also the possibility that someone may try to cause a denial of service. It's fairly unlikely that anyone could exploit a buffer overflow (especially in qmail), but certain other mail servers can't handle a long subject line appropriately.

    If I were really paranoid, I'd log information on who is using this -- remote host, remote IP address, and so forth. That way, you can see who's attempting to send spam through your servers. As it is, you only have your server logs to see who's hitting you repeatedly.

    This may sound awfully paranoid, but you really ought to remember rule #1: spammers cheat.

Re: Insecurities in my scripting
by PodMaster (Abbot) on Nov 21, 2002 at 03:00 UTC
    ... rude ... though I'm new ...

    I gotta say the easiest way to get good advice is to swallow your pride. What's a little abuse in exchange for great tips? Nevermind

    Why did you include %{} around your call to vars?

    %form = %{$query->Vars}; # perl -MCGI=Vars -e" %a=Vars();print for keys %a " a=b c=d # perl -MCGI=Vars -e' %a=Vars();print for keys %a ' a=b c=d
    I'd assume because you didn't know what the documentation clearly states (i know there is a ton of it, but we're only interested in the functions we use)
    Many people want to fetch the entire parameter list as a hash in which the keys are the names of the CGI parameters, and the values are the parameters' values. The Vars() method does this. Called in a scalar context, it returns the parameter list as a tied hash reference. Changing a key changes the value of the parameter in the underlying CGI parameter list. Called in a list context, it returns the parameter list as an ordinary hash. This allows you to read the contents of the parameter list, but not to change it.
    if ($webmaster eq "") {
    I see a lot of people coming from javascript doing this type of stuff, but it's utterly unneccessary. if($webmaster) would suffice, if all you required is a true value. Consider this example instead of that chain of if/else statements
    ### Make sure things are being completed, die slackers! my %youMustProvide = ( message => "Only a severe slacker would try to submit a form without l +eaving a message!\n", usermail => "Ok, buddy, how do you expect me to email you back if you +FORGET to leave yoru email address?!?\n", username => "Please click back and type in your name so I can spy on y +ou!\n", ); for my $key(keys %youMustProvide ) { # unless the parameter is present, and has a true value unless(exists $form{$key} and $form{$key} ) { # notify the user of his *forgetfulness* print $youMustProvide{$key}; # and do whatever else you need to in such an instance } }
    Some folks call it a "dispatch" table, and use it do do more than merely printing an error message, like providing regex patterns or callbacks for argument validation. There's probably even a module to do something like this (i know there is, but the name escapes me at the moment).

    Anyways, I hope you got something from this.

    ____________________________________________________
    ** The Third rule of perl club is a statement of fact: pod is sexy.

      Thanks so much for your help! I will reread that like a million times til i figure it out, I think I might try your dispatch table, that seems a lot neater than what I typically do.

      Thanks again!!

      sulfericacid
Re: Insecurities in my scripting
by Ananda (Pilgrim) on Nov 20, 2002 at 08:53 UTC

    Pretty good for a beginer!!

    yep, your are allowing any user visiting your form to edit and submit mails. This needs a work around, user may be allowed to communicate via, say, a questionire (using radio/check boxes..).

    Security is indeed needed.

    Ananda

    A reply falls below the community's threshold of quality. You may see it by logging in.