calling update_option("siteurl", $newval) can overwrite it with an instance of WP_Error.

[pbogdan]

Hello,

under admittedly somewhat obscure circumstances a call to
update_option("siteurl", ...) can overwrite its value with an instance of
WP_Error class.

When being updated siteurl is subject to some additional checks in
sanitize_option() function, and the bug is due its handling of WP_Error's.

In particular the code at
​https://github.com/WordPress/WordPress/blob/4.3.1/wp-includes/formatting.php#L3609-L3612:

• overwrites the $value variable with result of the call to $wpdb->strip_invalid_text_for_column() -
• will only create the $error variable if the WP_Error received from the call to $wpdb->strip_invalid_text_for_column() contains the optional error message

If $wpdb->strip_invalid_text_for_column() returns an instance of WP_Error
without an error message we end up with $value containing a WP_Error instance
but empty $error variable.

Then the check at
​https://github.com/WordPress/WordPress/blob/4.3.1/wp-includes/formatting.php#L3720
fails due to empty $error, and the WP_Error $value propagates upwards as the
return result of sanitize_option(), to be written into the database. As most
places trust get_option("siteurl") to return a string rather than an object this
results in fatal errors as WP_Error can't be converted to a string.

In the specific case I observed, the WP_Error instance without an error message
was originating from wpdb::get_table_charset() function -
​https://github.com/WordPress/WordPress/blob/4.3.1/wp-includes/wp-db.php#L2306-L2308. From
what I gather wpdb::strip_invalid_text_for_column() needs to know the database
charset for the database column to determine valid input, this will eventually
call out to wpdb::get_table_charset() and if the query in that function fails
for some reason the resulting WP_Error propagates all the way up to
sanitize_option().

As this was tracked down retrospectively I don't know what exact conditions
allowed the query in wpdb::get_table_charset() to fail while allowing any
subsequent queries to succeed, however it can be reproduced with a test case at
​https://gist.github.com/pbogdan/9827a8f2358cf9e8ca71

I have not reviewed the logic for other options handled by sanitize_option()
but it's possible some of them could be affected if they use the same logic for
dealing with WP_Error's - which relies on the WP_Error having the optional error
message.

I might be bit slow to respond but let me know if there is any additional
information I can provide.

Thanks,
Piotr

[MikeHansenMe]

Actually that won't work either because the $value still gets assigned the error and gets set in the option.