4
\$\begingroup\$

TL;DR

This month I wanted to improve my WordPress knowledge and create a simple form that wasn't bloated with features but was secure. I wanted to make sure this plugin improved my knowledge of:

  • reCaptcha
  • Akismet
  • AJAX
  • RESTful form submission

After spending the beginning part of the month figuring out WP_REST_Request I was able to create a form plugin. The plugin works and emails who the author of the page is but I was wondering if I could make any improvements to my PHP and if any of my code has a vulnerability:

The PHP:

if (!defined('ABSPATH'))
    die('-1');


function contact_js() {
    if (is_page_template('page-contact.php')) {
        wp_enqueue_script( 'contact_js', site_url() . '/js/contact.js', array('jquery'), '1.0.0', true);
        // URL STRING
        $localized = array(
            'url_string' => site_url(),
        );
        wp_localize_script('contact_js', 'url_object', $localized);
    }
}
add_action('wp_enqueue_scripts', 'contact_js');


function reCaptcha_in_head() {
    if (is_page_template('page-contact.php')) {
        ?><script src='https://www.google.com/recaptcha/api.js'></script><?php
    }
}
add_action('wp_head', 'reCaptcha_in_head');


add_action('rest_api_init', function() {
    register_rest_route('darthvader/v2', '/contact/', array(
        'methods'   => 'POST',
        'callback'  => 'darth_contact_form',
    ));
});


function darth_contact_form(\WP_REST_Request $request) {
    $pre_checks = true;
    if (check_ip_address() == "undefined") :
        $pre_checks = false;
    else :
        $form_ip_address = check_ip_address();
    endif;
    if (check_referrer() == "undefined") :
        $pre_checks = false;
    else :
        $form_referrer = check_referrer();
    endif;
    if (check_user_agent() == "undefined") :
        $pre_checks = false;
    else :
        $form_agent = check_user_agent();
    endif;
    if (akismet_verify_key(akismet_get_key()) != 'valid') :
        $pre_checks = false;
    endif;

    if ($pre_checks !== true ) {
        return new WP_Error('error', 'prechecks failed', array('status' => 400));
    }

    $form_email     = clean_string($request['form_email']);
    $form_name      = clean_string($request['form_name']);
    $form_message   = clean_string($request['form_message']);

    $recaptcha = reCaptcha_check($form_ip_address);
    if ($recaptcha !== true) {
        // return('Recaptcha shows this to be: ' . $recaptcha);
        return new WP_Error('error', 'reCaptcha says spam', array('status' => 400));
    }

    $akismet_outcome = akismet_spam_check($form_ip_address, $form_agent, $form_referrer, $form_email, $form_name, $form_message);
    // return('Is it spam? Akismet says: ' . $akismet_outcome);
    if ($akismet_outcome !== "false") {
        return new WP_Error('error', 'reCaptcha says spam', array('status' => 400));
    }

    $form_success = email_the_message($form_ip_address, $form_agent, $form_referrer, $form_email, $form_name, $form_message);
    if (false === $form_success) {
        return new WP_Error('error', 'email failed to send', array('status' => 400));
    }
    return;
}


// CHECK AKISMET IF IT'S SPAM
function akismet_spam_check($form_ip_address, $form_agent, $form_referrer, $form_email, $form_name, $form_message) {
    if (function_exists('akismet_http_post')) :
            global $akismet_api_host, $akismet_api_port;

            // CALL TO COMMENT CHECK
            $contact_data = array(
                'blog'                  => site_url(),
                'user_ip'               => $form_ip_address,
                'user_agent'            => $form_agent,
                'referrer'              => $form_referrer,
                'comment_type'          => 'contact-form',
                'comment_author'        => $form_name,
                'comment_author_email'  => $form_email,
                'comment_content'       => $form_message,
                'permalink'             => the_permalink(),
                'is_test'               => TRUE, // for testing purposes
            );

            // CONSTRUCT THE QUERY STRING
            $query_string = http_build_query($contact_data);

            // POST IT TO AKISMET
            $response = akismet_http_post($query_string, $akismet_api_host, '/1.1/comment-check', $akismet_api_port);

            // CHECK THE RESULTS
            $result = (is_array($response) && isset($response[1])) ? $response[1] : 'false';

            return $result;
    else :
        return new WP_Error('error', 'Akismet HTTP not found', array('status' => 404));
    endif;
}


// CHECK WITH RECAPTCHA
function reCaptcha_check($form_ip_address) {
    try {
        $url    = 'https://www.google.com/recaptcha/api/siteverify';
        $data   = [
            'secret'   => CAPTACHA_SECRET_KEY, // declared constant in wp-config
            'response' => $_POST['g-recaptcha-response'],
            'remoteip' => $form_ip_address,
        ];
        $options = [
            'http' => [
                'header'  => "Content-type: application/x-www-form-urlencoded\r\n",
                'method'  => 'POST',
                'content' => http_build_query($data)
            ]
        ];
        $context    = stream_context_create($options);
        $result     = file_get_contents($url, false, $context);
        return json_decode($result)->success;
        }
    catch (Exception $e) {
        return null;
    }
}


// TIME
function form_time() {
    $time = clean_string(date("F jS Y, h:ia", time()));
    return $time;
}


// REFERRER
function check_referrer() {
    if (isset($_SERVER['HTTP_REFERER'])) :
        $referer = clean_string($_SERVER['HTTP_REFERER']);
    else :
        $referer = "undefined";
    endif;
    return $referer;
}


// USER AGENT
function check_user_agent() {
    if (isset($_SERVER['HTTP_USER_AGENT'])) {
        $agent = clean_string($_SERVER['HTTP_USER_AGENT']);
    } else {
        $agent = "undefined";
    }
    return $agent;
}


// IP ADDRESS
function check_ip_address() {
    if (isset($_SERVER['REMOTE_ADDR'])) :
        $ip_address = clean_string($_SERVER['REMOTE_ADDR']);
    else :
        $ip_address = "undefined";
    endif;
    return $ip_address;
}


// SANITIZE
function clean_string($string) {
    $string = rtrim($string);
    $string = ltrim($string);
    $string = htmlentities($string, ENT_QUOTES); 
    $string = str_replace("n", "<br>", $string);
    if (get_magic_quotes_gpc()) {
        $string = stripslashes($string);
    }
    return $string;
}


function email_the_message($form_ip_address, $form_agent, $form_referrer, $form_email, $form_name, $form_message) {
     global $post;
    $to         = get_the_author_meta('user_email');
    $subject    = the_title();
    $body = 
        "TIME: "        . form_time()       . "\n" .
        "IP ADDRESS: "  . $form_ip_address  . "\n" . 
        "USER AGENT: "  . $form_agent       . "\n" .
        "EMAIL: "       . $form_email       . "\n" .
        "NAME: "        . $form_name        . "\n" .
        "MESSAGE: "     . $form_message     . "\n";
    // send the awesomeness
    wp_mail($to, $subject, $body);
}

After any improvements are suggested I will break up the PHP into a plugin and utilize WP Settings and Options API.

I did have help with this when I hit roadblocks. If you'd like to see where my faults lied here is a resource list of the Q&As I made to get me to this point:

\$\endgroup\$

1 Answer 1

1
\$\begingroup\$

Obviously this post is nearly four years old. Perhaps you’ve learned a few things in that time. Alas below are some suggestions. I don’t see any major issues- mostly a few places that could be simplified.

Calling validation functions multiple times

In function darth_contact_form() the check_* functions are called once in a conditional, and then if they don't return "undefined" they are called again. Instead of calling them twice, they could be called once,

if (($form_ip_address = check_ip_address()) == "undefined") :
    $pre_checks = false;
endif;
if (($form_referrer = check_referrer()) == "undefined") :
    $pre_checks = false;
endif;
if (($form_agent = check_user_agent()) == "undefined") :
    $pre_checks = false;
endif;

And really those check_* functions are a bit repetitive. A single function could be used for all three if it accepts the key to access into the super global.

function check_server_var($key) {
    if (isset($_SERVER[$key])) :
        $value = clean_string($_SERVER[$key]);
    else :
        $value = "undefined";
    endif;
    return $value;
} 

And it could return early which would eliminate the need for else:

function check_server_var($key) {
    if (isset($_SERVER[$key])) :
        return clean_string($_SERVER[$key]);
    endif;
    return "undefined";
} 

The logic could be reversed to allow the longer line to have fewer indentation levels

function check_server_var($key) {
    if (!isset($_SERVER[$key])) :
        return "undefined";
    endif; 
    return clean_string($_SERVER[$key]);
} 

Maybe it would be simpler to return false if those values are not set- then the calling code can simply check for false instead of the string ”undefined” but be sure to use equality checking if comparing with false.

Array syntax

In function reCaptcha_check() the array $data is declared using short array syntax, yet most other arrays use the longer traditional syntax. All arrays could be declared with the short array syntax.

Closing tags vs echo

In function

?><script src='https://www.google.com/recaptcha/api.js'></script><?php

instead of closing the PHP tabs one could use echo:

echo "<script src='https://www.google.com/recaptcha/api.js'></script>";

Or the short-echo tags:

   <?= "<script src='https://www.google.com/recaptcha/api.js'></script>" ?>
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.