WordPress.org

Make WordPress Core

Opened 6 years ago

Last modified 6 months ago

#27421 reopened defect (bug)

Escape madness in meta API

Reported by: Denis-de-Bernardy Owned by:
Milestone: Priority: normal
Severity: normal Version: 3.8.1
Component: Options, Meta APIs Keywords: has-patch
Focuses: Cc:
PR Number:

Description

I was trying to come up with a reasonably sane wrapper around the WP meta API so as to no longer need to call it directly, and I'm somewhat bemused by the slashing insanity in there...

As I read through the code:

  • get_post_meta() expects key and value to be unslashed
  • add_post_meta(), update_post_meta() and delete_post_meta() expect key and value to be slashed
  • WP uses wp_unslash() to strip slashes in add_metadata(), update_metadata(), and delete_metadata()
  • the codex suggests to use wp_slash() to add slashes before storing data

This could all work in the best of worlds, but then:

  1. the codex is mysteriously silent on the need to slash keys as well
  2. wp_unslash() will dutifully unslash objects, but wp_slash() doesn't slash them to begin with
  3. wp_slash() and wp_unslash() would not be able to slash private variables (short of using Reflection, perhaps) even if they wanted to
  4. wp_slash() and wp_unslash() will not bother processing array keys that could have been sent from e.g. a POST request.

Admittedly, it's not every day where $_REQUEST keys are inserted as is the database, so there's likely little room for SQL injections with respect to that last point, and array keys are typically without slashes, leaving little room for quirky keys that lose their slashes while getting stored.

Still, I'm left scratching my head as to if/when we'll actually allow objects to get stored using the meta API without running the risk of seeing their data unslashed at arbitrary points in time.

Attachments (1)

formatting.php.patch (1.3 KB) - added by jhorowitz 4 years ago.
Make wp_slash mirror wp_unslash (i.e. work on objects) v4.4.0+

Download all attachments as: .zip

Change History (9)

#1 @Denis-de-Bernardy
6 years ago

Oh yes, and I forgot what is possibly the most colorful point: update_post_meta() expects $meta_value to be slashed, but $prev_value is expected to be *unslashed*.

#2 @nacin
6 years ago

  • Milestone Awaiting Review deleted
  • Resolution set to duplicate
  • Status changed from new to closed

#3 @Denis-de-Bernardy
6 years ago

  • Resolution duplicate deleted
  • Status changed from closed to reopened

I'd argue this ticket has more to do with making:

  • wp_slash(wp_unslash($data)) and
  • wp_unslash(wp_slash($data))

work properly, meaning returning $data in each case irrespective of whether it's a scalar, a string, an array, an object, or a combination of the latter.

#18322 is more about a long term thing which, frankly, I don't expect to see fixed any time soon. We need wp_slash() to mirror wp_unslash() in the meanwhile, meaning wp_slash() must be capable of slashing objects.

Last edited 6 years ago by Denis-de-Bernardy (previous) (diff)

#4 @SergeyBiryukov
6 years ago

  • Milestone set to Awaiting Review

#6 @chriscct7
4 years ago

  • Keywords needs-patch added

@jhorowitz
4 years ago

Make wp_slash mirror wp_unslash (i.e. work on objects) v4.4.0+

#7 @jhorowitz
4 years ago

  • Keywords has-patch added; needs-patch removed

#8 @lopo
3 years ago

Any news on this?
I've stumbled on it when trying to fix compatibility issue between my plugin (Duplicate Post) and some visual editors that store objects in metadata: if I call wp_slash on the original post's custom fields before storing them into the copy they become unusable.

The patch seems to work fine. What's needed for that to make it into the core? Can I help in any way?

Note: See TracTickets for help on using tickets.