3

I need to make sure I'm not opening up a security hole in my site. I have a Custom Post Type called "Communities" and a custom post type called "Floor Plans" and need to add a parameter to the URL on Single Floorplan posts.

My goal is that when a Floorplan post object is selected from a Communities Single post, it appends the name of the community (i.e. single community post title) to the URL for use on the following page. So, enabling query vars:

    <?php

    //* Enable queryvars (functions.php)

    add_filter('query_vars', 'parameter_queryvars' );
    function parameter_queryvars( $qvars )
    {
    $qvars[] = "community";
    return $qvars;
    }

I have a template part called "dynamic-floor-plans" that lives in a loop at the bottom of Single Communities posts. I'm grabbing the single_post_title of the current communities page, and passing that to the URL so that it's available using $_GET

The reason we opted for this method is that the "Floor Plans" post type has a canonical/default version that has the same properties as its Community-specific counterparts. In order to avoid a post relationship situation (which would give the user flat out wrong data in some cases) the community versions just have to be able to use these query strings in very specific instances.

  <?php

  //* dynamic-floor-plans template part (placed in loop on communities page)

  global $post;
  $string = strtolower(single_post_title( null, false ));
  $slug = str_replace(' ','-',$string);
  ?>

     <div class="card-button">
      <a class="small expanded secondary button" 
        href="<?php echo get_the_permalink() . '?community=' . $slug ?>">View Floorplan</a>
   </div>

Now, this totally is working. I'm able to grab that community variable on the subsequently-selected floorplan single page using $_GET and things seem to be working nicely on the front end:

    <?php
    //*  Single Floorplans Template

    $community = $_GET["community"];

    ?>
      <h1 class="entry-title"><?php echo $title ?></h1>
      <h4>
        <?php if ($community) { ?> 
          At <?php echo $community; }  ?>
     </h4>
     ...

I just can't help but feel that this isn't kosher, or that there's a more secure way to do this.

Am I open to exploitation? I can include the function for the loop itself as well, but figure this code will be sufficient to get started in evaluation.

1
3

Passing non-private/non-protected/non-sensitive values through the URL is quite widely used and a more reliable way of passing values from one page to another. The reason for this is, $_SERVER['HTTP_REFERER'] is totally unreliable and can never be trusted. It can also, in many case be an empty value. Check the two following posts for more details

You can make use of cookies as well, but with current law, and also something controlled on user side, it also isn't mush better than $_SERVER['HTTP_REFERER'].

Just a note, WordPress pass a lot of query vars through the URL in order to know which data to display on which page. You should however note that, while passing value through the URL is very reliable, it can be compromised and should NEVER EVER be treated safe. Hackers like to pass malicious code through the URL, if a hacker pass malicious code together with your custom query var, and you use that as is, your site and/or db can be compromised in a really bad way.

Don't get me wrong here, using the $_GET global variable to pass data is really good way to pass data between two pages, and there is nothing wrong with that. You need to remember this though, any user submitted data through ANY type of forms, text areas, URL's, cookies, basically anything which requires any type of input or any data coming from client side apps, can be corrupted, and most probably is. You must never ever trust anyone, not even yourself. ALL INCOMING DATA must ALWAYS be sanitized, validated and/or escaped according to data type expected. This should be your number one priority and the top golden rule right on top of your list.

For as long as you keep to that one golden rule, you will be relatively safe, and I say relative safe as no piece of code will ever be secure. Sanitizing data and validating it against something static (if you can) before actually acting on that data will protect you against many "soft" attacks.

WordPress has lot of build in functions to take care of sanitation, escaping and validating data according to your needs and data type expected. Be sure to check this article in the codex on this topic. Alternatively, there are also filters available in PHP. For super globals, I tend to go with filter_input() as it takes care of checking whether the var is set, and if set, returns the value. You also have the ability to filter/sanitize the value

EXAMPLE

$community = filter_input( 
    INPUT_GET,             // Super global to use, in this case $_GET
    'community',           // The var to get a value from
    FILTER_SANITIZE_STRING // Type of filter to apply, here sanitize the value as a string
);

if ( $community ) {
    /**
     * $community have a value, so $_GET["community"] is set. 
     * It is also safe to use now as the value was sanitized when we
     * queried the value with filter_input
}
1
  • 1
    This is precisely what I was looking for. Thank you for the primer on sanitizing data, the example and the overall thoughtful response. This is one of those cornerstone sort of moments where problem meets explanation meets solution. I'll obsess over this for the next few months, I'm sure. Thanks again. – Jon Mar 29 '16 at 23:44

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.