4

I am working on a plugin that requires on-the-fly manipulation of content output. This is solely dependent on the current $_GET variable or $_REQUEST variable.

Depending on what the variable is set to, it will call a certain class method to handle the user's request and display the proper content.

I am fully aware of the Data Validation page on the WordPress Codex, however, I am unsure of what the best approach is for my scenario, or any scenario of sanitizing $_GET variables or $_REQUEST variables for that matter.

How can I sanitize using WordPress functions for $_GET variable or $_REQUEST variable for a string which will be matched to call a specific class method?

Could this be exploited or fail given the following code?:

public function display_admin_page(){
    if(is_admin() && isset($_GET['page'])){
        global $content;
        $page = sanitize_title($_GET['page']);
        $method_name = 'page_'.str_replace('-', '_', $page);
        if(method_exists('content', $method_name)){
            // Display requested page from content class
            $thePage = $content->$method_name();
        } else{
            $thePage = $content->error(404);    
        }
        echo $thePage;
    }
}
3

WordPress doesn't provide any specific data validation functions for SUPERGLOBALS.

I use the PHP filter_input function then escape it as I would any untrusted variable.

$url = filter_input( INPUT_GET, 'some_query_string', FILTER_VALIDATE_URL );

echo '<a href="'. esc_url( $url ). '">Click Me</a>';

The PHP filter input accepts:

2

To your specific example:

You sanitized the $_GET data appropriately (thought I would use sanitize_key instead of sanitize_title -- can't say there's much of a difference, but sanitize_title is intended for use in URLs).

The method_exists function will return true for private and protected methods, so if a user tries to call a private or protected method, it'll fail without going to the 404. (Unless the display_admin_page method is in the same class.)

That brings us to the main potential exploit: that absolutely anyone can force any public method in your class to run. If possible, it's always better to specifically whitelist what can be accepted. That way you could validate with something like:

if ( !in_array( $_GET['page'], array( 'accepted_method', 'another_accepted_method' ) ) )
     $content->error(404);
  • Could you kindly elaborate more on the potential exploit theory? – Michael Ecklund Sep 13 '12 at 14:15
  • Sure, I mean it's just something you have to think through. Every public method in your class can be forced to run by anyone at any time. That's not automatically an exploit, but it's a consideration. Is there a method in your class that deletes from your database? Is there a method that echoes your users' passwords? Will you ever add other methods you need to worry about? Etc. – SeventhSteel Sep 14 '12 at 13:23
0

I'd recommend using mysql_real_escape_string($_GET) on any GET request. It's a very powerful PHP feature.

You can then use str_replace() to replace any unwanted characters.

0

Sanitizing $_GET is pretty much context specific. Depends on what value you want and how you want it to be validated.

There is no one-sized fits all answer to this question. It is very context specific. For example, you could write a function that stripped all tags and slashes from input, that's very safe, but what if you wanted a p tag to be saved? No harm there. The wp_kses() family is an interesting study, but is not a great solution, as it takes into account context, user level and more. For example, as an admin user you can save JavaScript in post title and post content, but as a lower role, you can not.

If it's value is a known quantity, you can also check if in_array( $array_valid_strings ) and be extra sure about those.

That said, there are different degrees of sanitization, so it's important to keep your end goal in mind. I would consult this list and find the function or combination of functions that fits your needs. It's specifically the sanitize_ functions that should be used here, not esc_. Sanitization and escaping continue to be confused...

In my case I am going to use sanitize_text_field() because it sanitizes a string from user input or from the database.

  • Checks for invalid UTF-8,
  • Converts single < characters to entities
  • Strips all tags
  • Removes line breaks, tabs, and extra whitespace
  • Strips octets

Good luck :).

P.S. This answer quotes three different developer's perspectives (Josh, Michal, Kevin).

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.