I do some moonlighting on the side. Recently, I was asked to look over a script and either rewrite it or upgrade it. I took one look and told him to bring the script down and have me rewrite it. There are at least 2 major security flaws, 2 bugs, and at least 3 others I could smell. How many can you find?

#!/usr/bin/perl ################################# Read Me ########################## +#### # + # # Program: opt_in.cgi + # # + # # This program will + # # + # ###################################################################### +#### ############################# Path Variables ####################### +#### # Base path to program domain. $public = "/someplace/far/away"; # Carmine's address. $other= 'joe@schmoe.net'; # Name of footer file. $foot = "/foot.html"; # Name of header file. $head = "/head.html"; ###################################################################### +#### use CGI qw(:standard :html3); use DBI(); use Net::SMTP; use Socket; # If input is ok, proceed. if (aok()) { # Add client to database and send notification with opt out link. opt_in(); # If there's a comment give success page. if ($comment) { print "\n<p><b>Hello $name</b></P>\n<p>Thank you for your commen +t, it reads as follows:<br>\n<p>$comment</P>"; piece($foot); } # If no comment no content. else { print "Content-Type: text/html\nStatus: 204 No Content\n\n"; + }} # If there's a comment or extra fields mail them to gabe. if ($comment || $xtra) { mail_stuf('comment@webzen.com', $email, $subject, "$xtra$comment" +); # If this form is from webzen mail one to moe also. # if ($ENV{HTTP_REFERER} =~ m<^http://www\.someplace.com/.*>) { mail +_stuf($other, $email, $subject, "$xtra$comment"); } } exit 0; ###################################################################### +#### ###################################################################### +#### # Check the input data. sub aok { # If direct hit redirect to main site. if (!param()) { print "Location: http://www.someplace.com\n\n"; ret +urn undef; } # Get the local variables. $email = param('email'); $site = param('site'); $type = param('type'); $in = param('function'); $subject = "$site.com"; if (param('subject')) { $comment = "\nRe: "; $comment .= param('sub +ject'); $comment .= "\n\n"; } if (param('name')) { $comment .= "\nName: "; $comment .= param(' +name'); $comment .= "\n\n"; } if (param('comment')) { $comment .= param('comment'); } # Check for good data. if (!$site || !$type || !good_address($email)) { return undef; } # Add any extra info put on the form. if (param('xtra') eq "xtra") { xtra_stuf(); } # Get the IP address. $ip = remote_addr(); # Set the date. $date = `date '+%D'`; # Get this far, everythings ok. return 1; } ###################################################################### +#### # The paths to the particular header is built. sub build_paths { if (!$site) { $site = 'generic'; } $foot = "$public$site$foot"; $head = "$public$site$head"; } ###################################################################### +#### # Check for the existance of address. sub found { (my $dbh) = @_; my $ret_val = undef; my $sth; # Print sql statement for active list member. $sth = $dbh->prepare("select 'x' from opt_in where email=\'$email\' + and type=\'$type\'"); $sth->execute(); # If a row exists for this surfer return 1 (success). if ($sth->rows gt 0) { $ret_val = 1; $active = 1; } # Else print sql statement for inactive list member. else { $sth = $dbh->prepare("select 'x' from opt_in where email=\'$email\' + and type=\'NULL\'"); $sth->execute(); # If a row does not exist for this surfer return 2 (success). if ($sth->rows gt 0) { $ret_val = 1; }} # Clean up $sth->finish(); # Else return 1 (success). return $ret_val; } ###################################################################### +#### # The e-mail address is checked for an "@", a "." and no spaces. sub get_id { my $dbh = shift; # Prepare and execute sql statement. my $sth = $dbh->prepare("select id from opt_in where email=\'$email +\' and type=\'$type\'"); $sth->execute(); # Build and return id number.. my $ref = $sth->fetchrow_hashref(); $temp = 776869 + $ref->{'id'}; $sth->finish(); my $id = "012"; $id .= $temp; $id .= "345"; return $id; } ###################################################################### +#### # The e-mail address is checked for an "@", a "." and no spaces. sub good_address { my $email = shift; ((index($email,"@") ne -1) && (index($email,".") ne -1) && (index($ +email," " ) eq -1))? return 1: return undef; } ###################################################################### +#### # The results are mailed if selected (disabled). sub mail_stuf { (my $to, my $from, my $subject, my $comment) = @_; $comment =~ s/([\!|\#|\;])//g; # Connect to SMTP and mail. $smtp = Net::SMTP->new('mail'); $smtp->mail($from); $smtp->to($to); $smtp->data(); $smtp->datasend("To: <$to>\nSubject: $subject\n$comment"); # Close $smtp->dataend(); $smtp->quit(); } ###################################################################### +#### # The page for a remote opt in. sub opt_in { my $stmt; ($name, undef) = split(/@/, $email); # Connect to the database. my $dbh = DBI->connect("DBI:mysql:database=my_db;host=localhost", " +root", "", {'RaiseError' => 1}); if (!found($dbh)) { if (!$in) { $type = "NULL"; } $stmt = "insert into opt_in (email, type, domain, date, ip) values +(\'$email\', \'$type\', \'$site\', \'$date\', \'$ip\')"; } elsif ($in && $active) { $stmt = "update opt_in set domain=\'$site\ +', date=\'$date\', ip=\'$ip\' where (email=\'$email\' and type=\'$typ +e\')"; } elsif ($in) { $stmt = "update opt_in set domain=\'$site\', date=\'$ +date\', ip=\'$ip\', type=\'$type\' where (email=\'$email\' and type=\ +'NULL\')"; } if ($stmt) { $dbh->do($stmt); } # If successful print sucess message and mail confirmation. if ($comment) { build_paths(); piece($head); if ($in) { print "\n<P>Welcome $name, you have been added to<br> + \Someplace's opt-in mail list, The Foobar'ed.</P>\n<P>You will recei +ve an email confirmation.</P>\n<P>Thank you.</P>\n\n<P><a href=\"http +://www.someplace.com/\">Someplace</a></P><hr>"; }} if ($in) { $id = get_id($dbh); my $welcome = "\nHello, $name\n\nYou have been added to Someplac +e's opt-in elist, The FooBar'ed. \n\nThe FooBar'ed is published occas +sionally.\n\nIf this subscription is an error click below and you wil +l be removed.\nhttp://www.someplace.com/cgi-bin/foobar/option.cgi?$id +\n \nThank you\n\n--\nGabriel Zappia, President\nWebZen, Inc.\n(207) +799-5221\n\nhttp://www.someplace.com"; mail_stuf($email, "joe@boo.com", "FooBar'ed confirmation", $welc +ome); } # Clean up $dbh->disconnect(); } ###################################################################### +#### # A piec of html is grabbed from file and sent to stdout. sub piece { my $segment = shift; # Open the file. if (open (text1, $segment)) { # Prime line and print out the file. my $line = <text1> ; while ($line ne "") { print ($line,"\n") ; $line = <text1>; }} # Close and exit. close text1; } ###################################################################### +#### # This will concatenate any extra fields into an email to gabe. sub xtra_stuf { $xtra = undef; my %supres = ( "comment" => 1, "email" => 1, "type" => 1, "site" => 1, "xtra" => 1); ($name, undef) = split(/@/, $email); # If all is selected ignore the other 4 checkboxes. if (param('all')) { $supres{looksmart} = 1; $supres{odp} = 1; $supr +es{snap} = 1; $supres{yahoo} = 1; } # Reset the subject. $subject = "Inquiry from $name regarding $site.com"; # Build the extra fields string. for (param()) { if (!exists($supres{$_})) { $xtra_field = param($_); $xtra = "$xtra\n$_: $xtra_field"; }} # Return the xtra string. $xtra = "\n *** Original Message Follows *** \n$xtra\n\n"; } ###################################################################### +#### ###################################################################### +####

------
We are the carpenters and bricklayers of the Information Age.

The idea is a little like C++ templates, except not quite so brain-meltingly complicated. -- TheDamian, Exegesis 6

Please remember that I'm crufty and crochety. All opinions are purely mine and all code is untested, unless otherwise specified.


In reply to How many security holes can you find? by dragonchild

Title:
Use:  <p> text here (a paragraph) </p>
and:  <code> code here </code>
to format your post, it's "PerlMonks-approved HTML":



  • Posts are HTML formatted. Put <p> </p> tags around your paragraphs. Put <code> </code> tags around your code and data!
  • Titles consisting of a single word are discouraged, and in most cases are disallowed outright.
  • Read Where should I post X? if you're not absolutely sure you're posting in the right place.
  • Please read these before you post! —
  • Posts may use any of the Perl Monks Approved HTML tags:
    a, abbr, b, big, blockquote, br, caption, center, col, colgroup, dd, del, details, div, dl, dt, em, font, h1, h2, h3, h4, h5, h6, hr, i, ins, li, ol, p, pre, readmore, small, span, spoiler, strike, strong, sub, summary, sup, table, tbody, td, tfoot, th, thead, tr, tt, u, ul, wbr
  • You may need to use entities for some characters, as follows. (Exception: Within code tags, you can put the characters literally.)
            For:     Use:
    & &amp;
    < &lt;
    > &gt;
    [ &#91;
    ] &#93;
  • Link using PerlMonks shortcuts! What shortcuts can I use for linking?
  • See Writeup Formatting Tips and other pages linked from there for more info.