*_option() and *_transient() functions should expect unslashed data.

[Denis-de-Bernardy]

Following up on:

​http://core.trac.wordpress.org/ticket/9015#comment:136

It's totally irresponsible to expect plugin authors to escape whatever they send into get_option(). As things stand:

• get_option(), delete_option(), get_site_option() assume it's slashed
• add_option(), update_option(), add_site_option() seem to assume it's unslashed, as does get_option()
• *_transient() seem to expect unslashed input.
• delete_site_option() is very special: it expects slashed input if you're not on using multisite, but unslashed if you are
• update_site_option() is equallty special: it needs slashed input if the option is not loaded already, and unslashed input if it is

the list goes on, and on... these inconsistencies, which come on top of the *_meta() functions, which expect slashed data.

it's totally nuts, insecure, and irresponsible. especially considering calls in WP such as:

• get_option("{$size}_crop");

or functions such as:

function form_option( $option ) {
	echo esc_attr( get_option( $option ) );
}

function get_settings_errors( $setting = '', $sanitize = FALSE ) {
	global $wp_settings_errors;
	//... isn't it ironic that using sanitize here makes it LESS secure?
	if ( $sanitize )
		sanitize_option( $setting, get_option($setting));
	// ...

we're asking for trouble here...

[nacin]

I had noted some of this in #10788. I also discovered some odd multisite-dependent issues related to serialization, which were fixed there. I agree it is largely nonsensical, but how much can we do here?

[Denis-de-Bernardy]

Imo, we should expect unslashed input absolutely everywhere, even if it means introducing a few backwards compat issues in plugins from authors who know better.

We should also release WP 2.9.3 before SQL injection related hacks are all over the place. The number of potential loopholes related to this is too large for us to "wait for a worm to creep up". Especially if you consider that few plugin authors know that *_meta() expects slashed input. I take it that even fewer are aware that *_option() expects inconsistently slashed data.

[Denis-de-Bernardy]

See also #11956 while we're at it.

[Denis-de-Bernardy]

set_theme_mod() has an unsanitized option name

[Denis-de-Bernardy]

Here's one where we might be passing unslashed garbage straight into MySQL:

function maybe_add_existing_user_to_blog() {
	if ( false === strpos( $_SERVER[ 'REQUEST_URI' ], '/newbloguser/' ) )
		return false;

	$parts = explode( '/', $_SERVER[ 'REQUEST_URI' ] );
	$key = array_pop( $parts );

	if ( $key == '' )
		$key = array_pop( $parts );

	$details = get_option( "new_user_" . $key );
	add_existing_user_to_blog( $details );
	delete_option( 'new_user_' . $key );
	wp_die( sprintf(__('You have been added to this blog. Please visit the <a href="%s">homepage</a> or <a href="%s">login</a> using your username and password.'), site_url(), admin_url() ) );
}

[miqrogroove]

+1 as that code appears to be unconditionally hooked by ms-default-filters.php

[Denis-de-Bernardy]

untested

[Denis-de-Bernardy]

related: #11771

[ryan]

Given how inconsistent core WP itself is about slashing, I'm inclined to go with this. There will be some back-compat grief, but it's looking like more plugins are not escaping than are escaping. We'd be servicing the more popular expectation.

[Denis-de-Bernardy]

cool. I didn't do the meta functions in my patch. want me to go through those as well?

[nacin]

(In [13508]) Don't pass serialized values to hooks in options API. see #10788 see #12416

[Denis-de-Bernardy]

Replying to nacin:

(In [13508]) Don't pass serialized values to hooks in options API. see #10788 see #12416

ah, cool. that one got me scratching my head. :D

wpmuguru might know what that was about.

my patch needs a slight refresh because of this, too.

[ryan]

Refreshed patch

[ryan]

Did some light testing. Saved some options with quotes. Deleted all transients and let them regenerate.

[ryan]

(In [13673]) make *_option(), *_transient() functions consistently expect unslashed data. Props Denis-de-Bernardy. see #12416

[miqrogroove]

Notice: Undefined variable: _alloptions in /wp-includes/functions.php on line 518

[nacin]

(In [13677]) Fix typo in [13673]. see #12416

[nacin]

Do we plan do to this with _meta() as well?

[nacin]

(In [14074]) Revert changes to maybe_serialize() made in r13673. Do not prevent double-serialization of data. see #12930. see also #12416. xref #7347, r8100, r8372, and others.

[jamescollins]

In [13673], some $wpdb->prepare() calls were introduced that use '%s' instead of %s.

According to ​http://core.trac.wordpress.org/browser/trunk/wp-includes/wp-db.php#L856, these should be left unquoted.

As per ​http://core.trac.wordpress.org/browser/trunk/wp-includes/wp-db.php#L884), this won't actually cause problems, however I still think the instances of '%s' should be changed to %s in $wpdb->prepare() calls.

There are 3 instances of this in [13673].

[jamescollins]

Fixes the usage of %s in $wpdb->prepare() calls

[dd32]

(In [14211]) s/'%s'/%s/ in prepares. Props jamescollins. See r13673. See #12416

[nacin]

Expect unslashed in meta api

[nacin]

(In [14547]) Revert patch for expecting unslashed data in the metadata API. Had snuck in with [14546]. see #12416.

[nacin]

Patch added to include meta API in this.

[ryan]

We want to remove the stripslashes_deep() on the meta_values too, right?

[ryan]

We'll do meta in 3.1.

[hakre]

Any traction on this one? Is this still considered for 3.1?

[Denis-de-Bernardy]

#17346

[nbachiyski]

I would really love to see this committed and I would be happy to help.

Does the ticket has needs-patch label because the current patch is missing the part, which removes stripslashes for values?

[wonderboymusic]

We're ditching this in favor of #21767, correct?

[ryan]

Yes, we can pick this up there.

[nacin]

Closing this as fixed on 3.0 for *_option() and *_transient() functions.