Login/Install/User Edit should stripslashes() $_POST data

[dd32]

Following on from #13654 All Login/Registration/Install/User Edit functionality should stripslash $_POST data.

At present, it seems that we do not stripslash at all.

For existing user passwords, we should migrate passwords to their non-stripslashed versions:

[5/31/10 6:34:11 AM] Mark Jaquith: We could migrate people.

[5/31/10 6:34:13 AM] Dion (dd32): Perhaps oughta just add proper stripslashing in 3.1, and add back-compat to change password from non-stripslashed to stripslashed.. similar to the md5->phpass implementation..

[5/31/10 6:35:13 AM] Mark Jaquith: Yep. If the PW doesn't match, addslashes() and compare again. If that matches, set the new PW hash. Right?

[5/31/10 6:35:19 AM] Dion (dd32): yep

[johanee]

Attaching patch to fix this.

All paths for creating / editing users have been tested, including migration of un-stripslashed passwords.

Not directly regarding this issue:
Slashes handling in wp_insert_user, wp_create_user, + callers is rather convoluted (though less so for passwords compared to other fields).

It took more than one read-through of the code to convince myself that user name handling is valid (it all get fixed by a strict sanitize_user() in the end), and I'm still not entirely certain that the user_meta fields could not somehow get it wrong.

It could do with some reorganization, really.

[johanee]

Fix password handling to correctly stripslash()

[johanee]

Posted from Wordcamp Stockholm

I've gone back and thought more about the best way to implement this. I also found a bug in current patch -- will update.

My first solution stripslash passwords on input. This differs from how strings are generally handled: keep them slash'ed until the very end.

It is possible to delay stripslashing until calls to wp_hash_password(). This corresponds with how other wp_insert_user() strings are handled.

I've made another version implementing this.

It does however raise a question: what password version should be sent to the authenticate filters: slashed (like its always been) or stripslashed (which might be more intuitive)?

In the new version of the patch I've left it as-is (slashed for the filters). In the original version it is stripslashed. For variety!

Please comment if any of these approaches is accepatable.

[johanee]

Fixed patch using approach 1 (stripslash password input at once)

[johanee]

Approach 2, defer stripslash as long as possible

[johanee]

... and a final note:

I think this handles user changes in wp-admin/network/site-users.php correctly, but I'd love someone else to look at it.

[mdawaffe]

Note: This would also require a change to wp_xmlrpc_server::login() (or the escaping methodology in general of XML-RPC).