Enforce wp_slash() on functions that expect slashed data

[westonruter]

Since WordPress forces global input vars to get magic quoted, any access to them should pass through wp_unslash(). Likewise, any data data sent into a function that expects pre-slashed input should require an explicit wp_slash().

For example, wp_unslash() and wp_slash() could be enforced in situations like this:

$title = sanitize_text_field( wp_unslash( $_POST['title'] ) );
// ...
wp_insert_post( wp_slash( array(
    'post_title' => $title,
) ) );

As a WordPress-Extra rule, this will help enforce a discipline of unslashing, sanitizing, and slashing when slashing is required (e.g. in wp_update_post(), update_post_meta(), etc). It's easy to forget and for slashing to sneak in or to get stripped out, \\o/ o/, yay.

https://github.com/WordPress-Coding-Standards/WordPress-Coding-Standards/pull/395 implements the sniff for wp_unslash()

[remcotolsma]

The update_post_meta function will also stripslashes: http://codex.wordpress.org/Function_Reference/update_post_meta#Character_Escaping

So i'm not sure if all access to $_GET, $_POST, etc. should pass through wp_unslash().

[westonruter]

Yeah, you're right.

I'm tempted to sniff for wp_unslash() when accessing an input var, and then checking for wp_slash() when passing a variable into update_post_meta() and other functions that (ridiculously) expect pre-slashed data.

[remcotolsma]

Ok, that would also work for developers who are using the filter_input function.

I'm not sure it's the best approuch, i can imagine that it's sometimes easier to write things like:

update_post_meta( $post_id, 'test', $_POST['test'] );

It's indeed a pitty that this function expects slashed data. https://github.com/WordPress/WordPress/blob/3.9.1/wp-includes/meta.php#L151

[westonruter]

It is both a pity and a PITA.

[GaryJones]

Since WordPress forces global input vars to get magic quoted, any access to them should pass through wp_unslash()

I remember reading some of the discussions, but is there any documentation on the Codex or otherwise about this?

[JDGrimes]

Original ticket: https://core.trac.wordpress.org/ticket/21767 I couldn't find anything about it on the codex or developer handbook.

[JDGrimes]

I'm tempted to sniff for wp_unslash() when accessing an input var, and then checking for wp_slash() when passing a variable into update_post_meta() and other functions that (ridiculously) expect pre-slashed data.

Yeah, we'll add a separate sniff that checks that the data is slashed for all functions that expect slashed data.

[johnbillion]

Related: https://core.trac.wordpress.org/ticket/41593

[JDGrimes]

Note that I still have some old code for a sniff here, that I just haven't finished yet. The sniff itself can be found in this gist: https://gist.github.com/JDGrimes/7a29ec88d533459345565ae3caabe7d2

It needs to be updated before it can actually be added to WPCS, but it does contain some research in regard to functions that require slashing and ones that don't.

The list is incomplete, because basically what I've been doing is finding functions that require slashing, adding them to the list, and then running the sniff over core again. This reveals other functions that use those functions, and that therefore also require data passed to them to be slashed.

It is really a mess, and it would be nice if we could get this sniff finished so that in the future we can prevent additional functions in core from inadvertently requiring slashed data.

I'll try to take a whack at bringing it in line with the latest changes in WPCS soon.

[JDGrimes]

FYI, I'm working on this again. I hope to get an initial version finished this week. Currently I'm still analyzing core to find all of the functions that expect slashed arguments. It's a mess. I'll keep the discussion regarding that on the related trac ticket though.

[johnbillion]

It's a mess.

£10 says that you'll find a function which needs to accept either slashed or unslashed data depending on some logic inside the function.

[JDGrimes]

£10 says that you'll find a function which needs to accept either slashed or unslashed data depending on some logic inside the function.

Yes. In addition to functions that accept an array, part of which must be slashed, and part unslashed. I also just ran into a function that used to pass an arg through wp_unslash(), but no longer does. Still investigating why.

[jrfnl]

Was just thinking that, I'd definitely not take that bet. @JDGrimes You're a brave soul trying to untangle that mess. :heart:

[westonruter]

Yes, it\'s a very important effort!