Wordpress php Contact Form - Security flaws

Question

I have the following contact form included with my wordpress theme as a template.

Now a user on Stackoverflow pointed out that this form has some serious security vulnerabilities.

Could someone please point them out and tell me where I can improve

Form:

<?php
/*
 * Template Name: Contact Form Page
*/
if(isset($_POST['submitted'])) {
        //Check to make sure that the name field is not empty
        if(trim($_POST['contactName']) === '') {
            $nameError = __("You forgot to enter your name.", "site5framework");
            $hasError = true;
        } else {
            $name = trim($_POST['contactName']);
        }

        //Check to make sure sure that a valid email address is submitted
        if(trim($_POST['email']) === '')  {
            $emailError = __("You forgot to enter your email address.", "site5framework");
            $hasError = true;
        } else if (!eregi("^[A-Z0-9._%-]+@[A-Z0-9._%-]+\.[A-Z]{2,4}$", trim($_POST['email']))) {
            $emailError = __("You entered an invalid email address.", "site5framework");
            $hasError = true;
        } else {
            $email = trim($_POST['email']);
        }

        //Check to make sure comments were entered
        if(trim($_POST['comments']) === '') {
            $commentError = __("You forgot to enter your comments.", "site5framework");
            $hasError = true;
        } else {
            if(function_exists('stripslashes')) {
                $comments = stripslashes(trim($_POST['comments']));
            } else {
                $comments = trim($_POST['comments']);
            }
        }

        //If there is no error, send the email
        if(!isset($hasError)) {
            $msg .= "------------User Info------------ \r\n"; //Title
            $msg .= "User IP: ".$_SERVER["REMOTE_ADDR"]."\r\n"; //Sender's IP
            $msg .= "Browser Info: ".$_SERVER["HTTP_USER_AGENT"]."\r\n"; //User agent
            $msg .= "Referrer: ".$_SERVER["HTTP_REFERER"]; //Referrer

            $emailTo = ''.of_get_option('sc_contact_email').'';
            $subject = 'Contact Form Submission From '.$name;
            $body = "Name: $name \n\nEmail: $email \n\nMessage: $comments \n\n $msg";
            $headers = 'From: '.$name.' <'.$email.'>' . "\r\n" . 'Reply-To: ' . $email;

            if(mail($emailTo, $subject, $body, $headers)) $emailSent = true;

    }

}
get_header();
?>

            <div id="content" class="container clearfix">

                <!-- page header -->
                <div class="container clearfix ">



                    <?php if(of_get_option('sc_contact_map') != '') { ?>
                        <!-- contact map -->
                        <div id="contact-map">
                        <?php echo of_get_option('sc_contact_map') ?>
                        </div>
                        <!-- end contact map -->
                    <?php } else if(of_get_option('sc_showpageheader') == '1' &&  get_post_meta($post->ID, 'snbpd_ph_disabled', true) != 'on' ) : ?>

                        <?php if(get_post_meta($post->ID, 'snbpd_phitemlink', true)!= '') : ?>

                        <?php
                        $thumbId = get_image_id_by_link ( get_post_meta($post->ID, 'snbpd_phitemlink', true) );
                        $thumb = wp_get_attachment_image_src($thumbId, 'page-header', false);
                        ?>
                        <img class="intro-img" alt=" " src="<?php echo $thumb[0] ?>" alt="<?php the_title(); ?>"  />

                        <?php elseif (of_get_option('sc_pageheaderurl') !='' ): ?>

                            <?php
                            $thumbId = get_image_id_by_link ( of_get_option('sc_pageheaderurl') );
                            $thumb = wp_get_attachment_image_src($thumbId, 'page-header', false);
                            ?>
                            <img class="intro-img" alt=" " src="<?php echo $thumb[0] ?>" alt="<?php the_title(); ?>"  />

                        <?php else: ?>

                            <img class="intro-img" alt=" " src="<?php echo get_template_directory_uri(); ?>/library/images/inner-page-bg.jpg" />

                        <?php endif ?>
                    <?php endif ?>

                </div>


                <!-- content -->
                <div class="container">

                    <h1><?php the_title(); ?> <?php if ( !get_post_meta($post->ID, 'snbpd_pagedesc', true)== '') { ?>/<?php }?> <span><?php echo get_post_meta($post->ID, 'snbpd_pagedesc', true); ?></span></h1>

                    <article id="post-<?php the_ID(); ?>" <?php post_class('clearfix'); ?> role="article">

                        <?php if (have_posts()) : while (have_posts()) : the_post(); ?>

                            <div class="page-body clearfix">
                                <?php the_content(); ?>
                            </div>


                            <div class="one-third">
                                <div class="caddress"><strong><?php _e('Address:', 'site5framework') ?></strong> <?php echo of_get_option('sc_contact_address') ?></div>
                                <div class="cphone"><strong><?php _e('Phone:', 'site5framework') ?></strong> <?php echo of_get_option('sc_contact_phone') ?></div>
                                <div class="cphone"><strong><?php _e('Fax:', 'site5framework') ?></strong> <?php echo of_get_option('sc_contact_fax') ?></div>
                                <div class="cemail"><strong><?php _e('E-mail:', 'site5framework') ?></strong> <a href="mailto:<?php echo of_get_option('sc_contact_email') ?>"><?php echo of_get_option('sc_contact_email') ?></a></div>

                            </div>

                            <div class="two-third last">
                                <div id="messages">
                                    <p class="simple-error error" <?php if($hasError != '') echo 'style="display:block;"'; ?>><?php _e('There was an error submitting the form.', 'site5framework'); ?></p>

                                    <p class="simple-success thanks"><?php _e('<strong>Thanks!</strong> Your email was successfully sent. We should be in touch soon.', 'site5framework'); ?></p>
                                </div>

                                <form id="contactForm" method="POST">
                                    <div class="one-third">
                                        <label for="nameinput"><?php _e("Your name", "site5framework"); ?></label>
                                        <input type="text" id="nameinput" name="contactName" value="<?php if(isset($_POST['contactName'])) echo $_POST['contactName'];?>" class="requiredField"/>
                                        <span class="error" <?php if($nameError != '') echo 'style="display:block;"'; ?>><?php _e("You forgot to enter your name.", "site5framework");?></span>
                                    </div>
                                    <div class="one-third last">
                                        <label for="emailinput"><?php _e("Your email", "site5framework"); ?></label>
                                            <input type="text" id="emailinput" name="email" value="<?php if(isset($_POST['email']))  echo $_POST['email'];?>" class="requiredField email"/>
                                          <span class="error" <?php if($emailError != '') echo 'style="display:block;"'; ?>><?php _e("You forgot to enter your email address.", "site5framework");?></span>
                                    </div>
                                    <div class="two-third">
                                        <label for="nameinput"><?php _e("Area/Rep", "site5framework"); ?></label>
                                        <select>
                                            <option>Area 1 - Engela</option>
                                            <option>Area 2 - Francois</option>
                                            <option>Area 3 - Johan</option>
                                        </select>
                                    </div>
                                    <div class="two-third">
                                    <label for="Mymessage"><?php _e("Your message", "site5framework"); ?></label>
                                        <textarea cols="20" rows="20" id="Mymessage" name="comments" class="requiredField"><?php if(isset($_POST['comments'])) { if(function_exists('stripslashes')) { echo stripslashes($_POST['comments']); } else { echo $_POST['comments']; } } ?></textarea>
                                          <span class="error" <?php if($commentError != '') echo 'style="display:block;"'; ?>><?php _e("You forgot to enter your comments.", "site5framework");?></span>
                                    </div>
                                    <br class="clear" />
                                    <input type="hidden" name="submitted" id="submitted" value="true" />
                                    <button type="submit" id="submitbutton" class="button small round orange"><?php _e(' &nbsp;SEND MESSAGE&nbsp; ', 'site5framework'); ?></button>

                                </form>

                            </div>



                        <?php endwhile; ?>
                    </article>

                    <?php else : ?>

                    <article id="post-not-found">
                        <header>
                            <h1><?php _e("Not Found", "site5framework"); ?></h1>
                        </header>
                        <section class="post_content">
                            <p><?php _e("Sorry, but the requested resource was not found on this site.", "site5framework"); ?></p>
                        </section>
                        <footer>
                        </footer>
                    </article>

                    <?php endif; ?>


                </div>


            </div> <!-- end content -->

            <?php get_footer(); ?> 

Answer 1

And they didn't give ANY more detail? That was mean!

I think they meant cross site scripting attacks since your form does not check for the form's referrer, or remove html or JavaScript, and you echo out what the user has typed in that means they could add JavaScript to your site. Conceivably someone could set something on their website that posts to your form that executes JavaScript on your site; https://en.wikipedia.org/wiki/Cross-site_scripting

see this simple example

<form action='<?=$_SERVER['PHP_SELF']?>' method='post'>
<?php
   $value = (!empty($_POST['hello'])) ? $_POST['hello'] : '';
?>
<input type="text" name="hello" value="<?=$value?>" />
<input type='submit'/>
</form>

If I run that in a browser I get the box, I type in test and the value is "test" on page reload but if I type in

"><script>alert('danger!')</script>

note the "> which ends the input field allowing the code to run. now imagine if that was code that grabbed people's login cookies, or worse provided a login or registration form that sent your customers details to a spammer

as it happens google chrome tells me

Refused to execute a JavaScript script. Source code of script found within request.

and everything is safe... but people with older browsers beware!

Some other things to address

• as touched on above check the referrer is not coming from a remote site before sending the email; you might want to consider letting a blank referrer through for people with web privacy software but a referrer from some dodgy sounding hacker domain? ignore that submission

• you are not using wordpress' nonce feature, http://codex.wordpress.org/Function_Reference/wp_nonce_field that

The nonce field is used to validate that the contents of the form request came from the current site and not somewhere else. A nonce does not offer absolute protection, but should protect against most cases. It is very important to use nonce fields in forms

• I don't know of anything in particular that affects the php mail function, and it will depend on what server OS you are using, but there might be vulnerabilities in that similar to the JS problem, you might want to look into updates / know issues for your platform to make sure people can't force your server to send emails, a theoretical example if the name field is posted as some malformed string

  ^'; \r\n; bcc: [email protected]

could email anything to anyone; but I stress that is only theoretical, that syntax I invented - more research for your specific platform(s) is required.

• you are using eregi which has been deprecated a while in favor of the preg functions, I have not heard of any security issues with it (but then I haven't looked because I use preg), but best change because you might find it removed from php soon.

• your form does not check for maximum length of comment, so unless there is something at the webserver level to stop it, someone could post huge comments to you which would then jam up your email or your servers sendmail - never seen this happen, but still.

• you might want to consider using a service like http://akismet.com/ to protect your form from spammers (that may even protect you from a lot of the above)

Answer 2

if(function_exists('stripslashes')) {

OMG! You don't know if the function is available? You should. And you should never run it without checking whether you need to - which implies you don't know if you've got addslashes enabled - which you should never do for any reason.

This construct seems messy to me:

$nameError = __("You forgot to enter your name.", "site5framework");
$hasError = true;
...
$emailError = __("You forgot to enter your email address.", "site5framework");
$hasError = true;
...
if(!isset($hasError)) {

Wouldn't it be better to just use:

$formError[]= __("You forgot to enter your name.", "site5framework");
...
$formError[]= __("You forgot to enter your email address.", "site5framework");
...
if (!count($formError)) {

There's multiple XSS vulnerabilities due to you echoing un-sanitized content back out to the email content (if the email is viewed via a browser) and in the html sent to the browser. Also the isset() condition is adding nothing here (OK it suppresses a strict warning - but is that really worth the effort?). Instead of:

<input type="text" id="nameinput" 
   name="contactName" value=
   "<?php if(isset($_POST['contactName'])) echo $_POST['contactName'];?>"          
   class="requiredField"/>

Consider:

<input type="text" id="nameinput" 
   name="contactName" value=
   "<?php echo @htmlentities($_POST['contactName']);?>"          
   class="requiredField"/>

Your code is vulnerable to email header injection.

Also there is the opportunity to exploit MUA vulnerabilities since you don't force an encoding on the email body.

Also, even running on Wordpress, there's a chance this could be abused to launch a flood of emails on your server. If your MTA throttles outgoing emails, this could create a huge backlog. If it doesn't throttle emails you could get blacklisted due to volume. Either way you could use up any bandwidth quota.

Note that you should NEVER sanitize input. You should ALWAYS sanitize output.