sanitize_user_object() throws fatal error on user property objects

[filosofo]

Suppose a plugin has serialized an object and saved it as user-meta data. (I'm not saying this is a good idea, but other people are doing it).

get_user_to_edit() gets the user data as an object with that now-unserialized-object as a property of the user object. Then it passes the user object to sanitize_user_object().

sanitize_user_object() currently loops through each property of the user object and if the property is not an array, it attempts to sanitize it. When the property is an object, you get a fatal error trying to cast the object to a string.

My patch, rather than excluding arrays, only attempts to sanitize things that are strings or numbers.

You're probably thinking, "if checking for objects is the problem, why not just call is_object()?" The problem is that if the serialized object is of an object type that is no longer defined, is_object() returns false. This could happen in the case in which a plugin saves the serialized object as user meta data, and then the plugin is deactivated. Then you end up with something that's a __PHP_Incomplete_Class, a non-object object.

[filosofo]

This is one cause of the white screen of death for some people on their 2.9 profile pages.

[hakre]

That would need two checks, the array variant of the user might have such objects as well, hasn't it? (Line 650 ca.)

Addtionally the function "sanitize_user_object" has the problem that it does not return a user - if object - by reference as it is common practise in wordpress. But that's only a sidenote.

the function sanitize_user_object accepts by definition object related values only. this might be a documentation problem. if not, this should be properly reflected in function sanitize_user_object. sanitize_user_object is misleading as well because according to it's code and docblock, it accepts a user array as well.

sanitize_user_field renders useless in the raw context. It seems pretty useless that it is implemented then.

line 692 in sanitize_user_field() looks like the counterpart that you fix with your patch. it should be updated as well.

docblock of function sanitize_user_field is pretty redundant to the code, the @uses apply_filters() notes are pretty useless. better formatted php code does help a lot more than double code, one time in comments with pseudo code and second time in the function with real code.

for the line 692 topic I've created a second patch. I would not recommend this for 3.0 because as filosofo pointed out this actually breaks things.

[hakre]

Replying to filosofo:

You're probably thinking, "if checking for objects is the problem, why not just call is_object()?" The problem is that if the serialized object is of an object type that is no longer defined, is_object() returns false.

is_object() does not unserialize a value. it will return false. it will only return true on real (not serialized) objects.

[filosofo]

Replying to hakre:

is_object() does not unserialize a value. it will return false. it will only return true on real (not serialized) objects.

But in the case I mentioned the "object" is unserialized as an __PHP_Incomplete_Class object. It's not just a string of serialized data.

[filosofo]

hakre, your patch has a typo: "retirm"

[hakre]

Replying to filosofo:

Replying to hakre:

is_object() does not unserialize a value. it will return false. it will only return true on real (not serialized) objects.

But in the case I mentioned the "object" is unserialized as an __PHP_Incomplete_Class object. It's not just a string of serialized data.

Ah okay, I was not aware of that. I guess that's a nice case which shows where usage makes sense with user options. I suggest to base64_encode serialized values to protect their integrity within wordpress.

Replying to filosofo:

hakre, your patch has a typo: "retirm"

Thanks for reporting, fixed.

[Denis-de-Bernardy]

see also #9640 and related tickets

[ryan]

(In [12511]) Sanitize only string and numeric fields in the user object. Props filosofo hakre. fixes #11509 for trunk

[ryan]

[12512] for 2.9.