Take the 2-minute tour ×
WordPress Development Stack Exchange is a question and answer site for WordPress developers and administrators. It's 100% free, no registration required.

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;
    }
}
share|improve this question

3 Answers 3

up vote 2 down vote accepted

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:

share|improve this answer

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);
share|improve this answer
    
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

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.

share|improve this answer

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

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