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've been wondering about this for a while. Is the following if statement safe enough? Is it easy to break through the code and get access to the content when not logged in?

<?php if ( is_user_logged_in() ) { 
     // SECURE CONTENT
} else { 
     // LANDING PAGE
} ?>
share|improve this question

3 Answers 3

up vote 4 down vote accepted

Well, you have to ask yourself "Secure enough for what?" I doubt you are a bank or other institution that needs exceptionally high security. If you were you'd a team of $100,000+ per year experts to answer this question for you.

With that in mind...

You'd have to subvert the WordPress login system to get past that code. I am sure that is possible but if your passwords are solid it should be pretty difficult. It isn't going to be a "put a null-byte in the query string" kind of a hack. It should be secure enough for most purposes.

On thing to worry about is that both is_user_logged_in, and wp_get_current_user upon which it depends, are both pluggable (nonsensically so to my mind) meaning that a malicious plugin, or just a badly written one, could completely destroy your security.

share|improve this answer
    
To be fair it's not like if they weren't pluggable it would make a difference to malicious code... :) –  Rarst Jul 7 '13 at 22:40
    
To me, the login system is one system that should be difficult to alter. Sure, malicious could could always do damage, but I guess it is just how easy it is with pluggable functions. Getting "private" posts to display is harder than altering the login system. How can "private" posts possibly be more important? Not the place to argue that though :) –  s_ha_dum Jul 7 '13 at 22:56
    
@s_ha_dum IMO this is the place to argue that through :) Interesting question, lots of things to consider and many possible insights. –  kaiser Jul 8 '13 at 10:17
    
Hah... that little quip started something... ! –  s_ha_dum Jul 8 '13 at 13:29
    
Straight to the point. Thanks for a great answer. –  Frederick Andersen Jul 8 '13 at 21:53

In short - and for general use - yes. As @s_ha_dum pointed out though, it really depends on what you're using it for.

Is the function secure?

As @kaiser and @s_ha_dum have mentioned the function is pluggable - meaning that any plug-in (though not themes) can redefine it to do anything. But this isn't really a problem insofar as that plug-ins can do almost anything - they are executing code on your server.

The fact that its 'pluggable' is not really the issue here. You should ensure that all plugins - and themes - you use can be reasonably trusted. Where security is vital, companies will tend to create their own plug-ins, and only use those.

(The fact that its pluggable is so companies can implement a more secure authentication system if the need is there).

Is the method of authentication secure?

Let's assume that the function is defined as in WordPress core. Is it secure? (Yes).

The is_user_logged_in() function checks the user's browser for a valid cookie. If its not there, is invalid, or has expired - they are forced to log-in again.

The cookie is generated from the following:

  • User's username
  • 8 characters taken from their password hash
  • Timestamp for when the cookie expires

The cookee itself contains the username and timestamp in plain text. It also contains a HMAC (hash message authentication code) - this is part is difficult to guess, and depends on the username & timestamp. So if an attacker got hold of an expired cookie, or a cookie for their own account - they couldn't produce a valid cookie that would log-in them into an admin account without producing a valid HMAC.

The HMAC is produced from a hash of the above 3 elements. Now 8 characters doesn't seem much - that could be brute forced within a week. But in fact the hash is a salted hash - using your site's SALTs. With this, and so long as they are secure, any attempts at brute forcing are foiled.

... so really the security of that function lies in the security of your website's SALTs. These are stored in your wp-config.php, along with your database credentials. You should never make that public.

Can I detect if a function has changed the definition of a pluggable function?

Kaiser's function above works only if an MU-plugin has overwritten a function. Here's a more general solution, with the disclaimer that a plug-in not overwriting a pluggable function is not an indication that your plug-ins are secure.

/**
 * Do not use in production.
 * 
 * Checks if any plug-ins have overwritten a pluggable function in 
 * `wp-includes/pluggable.php` with a custom function.
 * 
 * Prints list and kills processing if a function has been overwritten.
 * 
 * Based on Kaiser's answer (link below).
 * @link http://wordpress.stackexchange.com/questions/105637/is-wordpress-is-user-logged-in-secure/105667#105667
 */
function wpse105637_was_pluggable_defined_by_plugin(){

    $arr = token_get_all( file_get_contents( ABSPATH.WPINC.'/pluggable.php' ) );
    $functions = array();
    foreach ( $arr as $curr_key => $part ){
        if ( ! is_array( $part ) )
            continue;

        if ( token_name( $part[0] ) === 'T_FUNCTION' ){
            while ( is_array( $arr[ $curr_key ] ) AND 'T_FUNCTION' !== $arr[ $curr_key ][0] )
                ++$curr_key;
            $functions[] = $arr[ --$curr_key ][1];
        }
    }

    $overwritten  = 0;
    foreach ( $functions as $f ){
        $reflFunc = new ReflectionFunction( $f );
        if( ABSPATH.WPINC.'/pluggable.php' !== $reflFunc->getFileName() ){
            $overwritten++;
            printf( '<h1> Functions Overwritten </h1>' );
            print '<code>'.$f .'</code> ' . $reflFunc->getFileName() . ':' . $reflFunc->getStartLine() . '<br/>';
        }
    }

    if( $overwritten )
        wp_die( sprintf( '%d pluggable functions overwritten', $overwritten ) );

}
add_action( 'plugins_loaded', 'wpse105637_was_pluggable_defined_by_plugin' );
share|improve this answer
    
+1 for "with the disclaimer that a plug-in not overwriting a pluggable function is not an indication that your plug-ins are secure" and the fix. –  kaiser Jul 8 '13 at 12:01
    
+1 for the "Is the method of authentication secure?" section. Really helpful. –  Frederick Andersen Jul 8 '13 at 21:56

Pluggable functions

First off, pluggables are loaded from within the ~/wp-settings.php file. They are included after all plug-in files have been included (but before plugins_loaded) so any plugin can replace them. (mu-plugins are included even earlier, so they take precedence).

Obstacles for a hack

The default user will never worry about this, as (s)he might not even have a muplugins folder, as it isn't added by core by default. So the first thing the "hacker" needs to do is to actually add the folder. This means that (s)he already has access to your file system and you got completely different problems than a hack on your installed System. No serious hacker would now bother with WordPress when every other, easier accessible door stands wide open. So just check if WPMU_PLUGIN_DIR and WPMU_PLUGIN_URL are defined. If they're not, you don't have to worry.

The next thing - in case you got a muplugins folder - you want to do is check those files. This is pretty easy: MU-Plugins only work as single file plugins. So if you got more plugins than files, than one of your files is already loading external files. You can easily monitor this with counting the total number of files with get_included_files() on the muplugins_loaded hook at priority 0, then diff the count against the included files again on the plugins_loaded-hook on priority 0. The difference must be equal to what you get back from

count( glob( WPMU_PLUGIN_DIR.'/*.php' ) );

The last thing to worry.

So let's say that someone really managed to hand you over a MU-Plugin (that you had to install by yourself after you added the folder yourself), then it's time to monitor if one of the pluggable functions were replaced. You can use below plugin to do so. It will simply die() in any case where one of your functions were replaced - "protecting" you and your visitors. It hooks in at the latest possible (there's no real "latest" in hook priority) time in, reads all the functions from the pluggable file and then checks for their existence. As it only grabs the content of the file, it doesn't get included and therefore those functions won't get defined. This way you can make sure, the MU-Plugin will always stay up to date, as the number of functions might change, but the plugin will still know about it.

Keep in mind that the plugins needs at least PHP version 5.3.

<?php
defined( 'ABSPATH' ) OR exit;
/**
 * Plugin Name: (#105637) Check pluggable changes
 */
add_action( 'muplugins_loaded', 'wpse_pluggable_check', pow( 99, 99 ) );
function wpse_pluggable_check()
{
    $arr = token_get_all( file_get_contents( ABSPATH.WPINC.'/pluggable.php' ) );
    $functions = array();
    foreach ( $arr as $curr_key => $part )
    {
        if ( ! is_array( $part ) )
            continue;

        $token = token_name( array_shift( $part ) );
        $token === 'T_FUNCTION'
            AND $functions[] = $arr[ $curr_key + 2 ][1];
    }
    foreach ( $functions as $f )
        function_exists( $f ) AND wp_die( 'pluggable overwritten!' );
}

Above plugin assumes that the function scheme always is function fn_name() {. Here's a slightly enhanced version that searches the following arrays.

$arr = token_get_all( file_get_contents( ABSPATH.WPINC.'/pluggable.php' ) );
$functions = array();
foreach ( $arr as $curr_key => $part )
{
    if ( ! is_array( $part ) )
        continue;

    if ( token_name( $part[0] ) === 'T_FUNCTION' )
    {
        while ( is_array( $arr[ $curr_key ] ) AND 'T_FUNCTION' !== $arr[ $curr_key ][0] )
            ++$curr_key;
        $functions[] = $arr[ --$curr_key ][1];
    }
}
foreach ( $functions as $f )
    function_exists( $f ) AND wp_die( 'pluggable overwritten!' );
share|improve this answer
    
+1 for giving me an insight in MU-plugins! –  Frederick Andersen Jul 8 '13 at 21:54

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.