tags:

views:

38

answers:

1

Hey guys, I have very basic php skills. As it stands, I'm trying to "reverse" engineer a contact form I found line that has some level security (I truthfully don't know how good it is). I think I got it, it works, but I just thought maybe some of you more experience in php can glance it over and see if it's actually still secure.

As I said, I reverse engineered it, so I don't know if I broke the security, but it does work just fine. As you can see it's pretty basic. I've been reading a lot and learning about what everything does, just worried about ordering and redundancy (want to avoid it if necessary)... basically anything I can do to clean it up as much as I can.

Many thanks guys :)

Code below:

<?php
            $to='[email protected]';
            $sender=stripslashes($_POST['sender']);
            $email=stripslashes($_POST['email']);
            $subject=stripslashes($_POST['subject']);
            $message=stripslashes($_POST['message']);
            $body=  "Greetings,\n\n$message\n\n$sender" .
                    "\n" ;

            $displayForm=true;
            if ($_POST){
                $sender=($_POST['sender']);
                $email=($_POST['email']);
                $subject=($_POST['subject']);
                $message=($_POST['message']);
                $valid=eregi('^([0-9a-z]+[-._+&])*[0-9a-z]+@([-0-9a-z]+[.])+[a-z]{2,6}$',$email);
                $crack=eregi("(\r|\n)(to:|from:|cc:|bcc:)",$sender);
                $crack=eregi("(\r|\n)(to:|from:|cc:|bcc:)",$message);
                    if ($sender && $email && $message && $valid && !$crack){
                    if (mail($to,$subject,$body,'From: '.$email."\r\n")){
                $displayForm=false;
        ?>
        <div>Your message has been sent successfully. Thank you for contacting us.</div>
        <?php
            echo '<p>'.htmlspecialchars($message).'</p>';
            }else {
        ?>
        <div>Your message could not be sent due to a system error. We apologize for any inconvenience.</div>
        <?php
            }
            }else if ($crack){
        ?>
        <div>Your message contained one or more anomalies, indicative of malicious content. Please consider revising your wicked ways.</div>
        <?php
            }else {
        ?>
        <div>You failed to complete a required field, or to provide a valid email address.</div>
        <?php
            }
                }
            if ($displayForm){
?>

<form action="./" method="post">
    <div class="contact-border"><input class="contact-textbox" type="text" name="sender" id="sender" tabindex="1" value="<?php echo htmlspecialchars($sender); ?>" /> <strong> Name</strong></div>

    <div class="contact-border"><input class="contact-textbox" type="text" name="email" id="email" tabindex="2" value="<?php echo htmlspecialchars($email); ?>" /> <strong> Email Address</strong></div>

    <div class="contact-border-noreq"><input class="contact-textbox-noreq" type="text" name="subject" id="subject" tabindex="3" value="<?php echo htmlspecialchars($subject); ?>" /> <strong> Subject (optional)</strong></div>

    <div class="contact-border"><textarea class="contact-textbox" name="message" id="message" cols="90" rows="15" tabindex="4" ><?php echo htmlspecialchars($message); ?></textarea></div>

    <div><input class="submit-button" type="submit" value="Submit" name="submit" /></div>
</form>
        <?php
            }
        ?>
A: 

Your code looks alright, but might I suggest adding a CAPTCHA? I made the mistake of not putting one up on my site, and paid a hefty price in spam for a couple days.

Jason B
Thanks a lot Jason. I had recaptcha before but took it off and adopted the "hidden field" option. I have an input that is hidden through css that will kill the script if it's filled in. I haven't had spam yet using it. I just hate filling out captchas and don't want to put my visitors through that.I removed it above to simplify the code and make it easier on anyone looking over it.I was hoping to protect myself from header injections or running scripts, highjacking, etc. Basically looking to make it as secure as possible.Thanks again for your help :)
WS