#11509 closed defect (bug) (fixed)
sanitize_user_object() throws fatal error on user property objects
Reported by: | filosofo | Owned by: | ryan |
---|---|---|---|
Milestone: | 2.9.1 | Priority: | normal |
Severity: | normal | Version: | 2.9 |
Component: | Users | Keywords: | sanitize_user_object has-patch |
Focuses: | Cc: |
Description
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.
Attachments (2)
Change History (12)
#3
@
15 years ago
- Milestone changed from 3.0 to 2.9.1
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.
#4
in reply to:
↑ description
;
follow-up:
↓ 5
@
15 years ago
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.
#5
in reply to:
↑ 4
;
follow-up:
↓ 7
@
15 years ago
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.
#7
in reply to:
↑ 5
@
15 years ago
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.
This is one cause of the white screen of death for some people on their 2.9 profile pages.