SQL Injection, escaping and magic quotes

  • As I understand it WordPress for some reason checks for magic quotes and if they are disabled manually adds slashes. What I want to find out is in that case can I assume that the database methods are aware that the data has already been escaped? Functions such as $wpdb->insert(), $wpdb->update(), $wpdb->prepare() and even $wpdb->escape(). Will these functions double escape certain characters such as single quotes? Looking at the $wpdb->escape() method from what I can tell it just escapes again. So in this case surely special characters such as ‘ are going to be double escaped when calling $wpdb->escape()?

    It’s interesting that in the codex documentation for the wp_kses() method it tells you that you need to remove any slashes from PHP’s magic quotes before you call this function. However no such warnings are present in the documentation for the database methods I listed above.

    So do we need to manually remove slashes when calling these database functions? How about functions like wp_kses_post() and wp_kses_data().

    It’s all very unclear as to the best practice to follow in WordPress.

Viewing 2 replies - 1 through 2 (of 2 total)

  • The WPDB funtions like insert() and update() are sanitised so you do not need to escape anything being given to those functions. The WPDB class takes care of all this for you, so you don’t need to escape anything beforehand.

    Functions like wp_kses_post() are not database related, so they don’t do any sort of database sanitisation for the content, and they shouldn’t either because they have nothing to do with the database.

    However, everything that’s sent via POST or GET is affected by settings like magic-quotes, The best solution is to turn it off on the server. It’s been recognised as more or a problem then a solution.

    This is another reason why PHP has moved away from the whole magic-quotes and safe-mode ideal that was popular in PHP3/4 and has gotten rid of it all (basiclaly) in PHP5. It causes a whole lot more problems then it fixes.

    Thread Starter peterbro80

    (@peterbro80)

    Yep I fully agree that magic quotes should be disabled. I understand that wp_kses_post() and related functions have nothing to do with the database. My issue is that it seems WordPress forces magic quotes. Turning it off in php.ini will not work for WordPress because WordPress detects that it has been turned off and manually adds it back in!

    See wp-includes/load.php…

    function wp_magic_quotes() {
	// If already slashed, strip.
	if ( get_magic_quotes_gpc() ) {
		$_GET    = stripslashes_deep( $_GET    );
		$_POST   = stripslashes_deep( $_POST   );
		$_COOKIE = stripslashes_deep( $_COOKIE );
	}

	// Escape with wpdb.
	$_GET    = add_magic_quotes( $_GET    );
	$_POST   = add_magic_quotes( $_POST   );
	$_COOKIE = add_magic_quotes( $_COOKIE );
	$_SERVER = add_magic_quotes( $_SERVER );

	// Force REQUEST to be GET + POST.
	$_REQUEST = array_merge( $_GET, $_POST );
}

    What it’s doing is checking for magic quotes and if they are enabled it strips them. Then it manually adds magic quotes. The net result is you end up with all $_GET, $_POST, $_COOKIE and $_SERVER data with slashes added regardless of any php settings.

    My question was more to do with when writing plugins do we have to then manually strip the slashes away before calling the various database functions? As you say the database functions handle escaping for us using placeholders. So in theory we should be calling stripslashes_deep on any data before passing it to these database functions.

    Upon investigating the code further it appears that yes we have to manually call stripslashes_deep on any input data which could contain escaped characters before we pass it to the $wpdb methods. I can confirm that WordPress actually does this itself in core code.

    In wp-includes/post.php inside wp_insert_post()
    $data = stripslashes_deep( $data );

    So it appears to me that as a general rule we should be calling stripslashes_deep() on any input data before calling the $wpdb methods. Also before calling wp_kses(), wp_kses_post() and wp_kses_data() we should be calling stripslashes_deep() on any data first.

    Of course this isn’t ideal, no idea why WordPress would be forcing magic quotes when as you say it is being phased out of PHP. Escaping should be done to data when outputting it to an external system (such as the database) using a method specific to that external system. Forcing magic quotes on all input data is making the assumption that the data is going to one place only, the database. Escaping should be done on output not input. Validation should be done on input.

Viewing 2 replies - 1 through 2 (of 2 total)
  • The topic ‘SQL Injection, escaping and magic quotes’ is closed to new replies.