WordPress.org

Make WordPress Core

Opened 3 years ago

Closed 10 months ago

#18786 closed enhancement (fixed)

meta_form() should place some restrictions on meta keys

Reported by: nacin Owned by: nacin
Milestone: 3.7 Priority: normal
Severity: normal Version:
Component: Administration Keywords: 2nd-opinion 3.7-early
Focuses: Cc:

Description

meta_form() echoes out all meta keys into a dropdown for the custom fields box, unless they start with an underscore (as bound by the query).

We should consider is_protected_meta( $key, 'post' ) and/or current_user_can( 'add_post_meta', $post->ID, $key ). This isn't a security thing, just an opportunity to hide some things from the user they don't need to see.

On the other hand, it's definitely a number of extra calculations. is_protected_meta() is light as long as there's no filter on things (and if there is, we probably want to know). current_user_can() might be a bit more weight than necessary here.

Change History (10)

comment:1 mdawaffe2 years ago

+1 for either. We shouldn't show is_protected_meta keys.

comment:2 macbrink18 months ago

+1 for at least is_protected_meta

If a filtered protected meta appears in the dropdown list, and a user selects and adds this field, he will see a very confusing message:

An unidentified error has occurred

comment:3 nacin18 months ago

  • Milestone changed from Awaiting Review to 3.6

comment:4 nacin17 months ago

In 23534:

Ignore protected meta keys in meta_form(). see #18786.

comment:5 georgestephanis16 months ago

Are we good to close this? The only thing I can think of in this would be whether it would eventually need to take custom post types into account for protected meta keys, but that's not an immediate concern if at all.

comment:6 nacin15 months ago

  • Keywords 3.7-early added
  • Milestone changed from 3.6 to Future Release

I'm still wondering if we should also use the current_user_can() checks. I haven't had time to profile this. Going to punt this to 3.7.

comment:7 wonderboymusic11 months ago

  • Milestone changed from Future Release to 3.7

these are all marked 3.7-early

comment:8 nacin10 months ago

#25189 was marked as a duplicate.

comment:9 nacin10 months ago

I never noticed that meta_form() actually has a LIMIT (of 30 by default) for the number of keys it queries. So that should mean we're not calling current_user_can() too much.

comment:10 nacin10 months ago

  • Owner set to nacin
  • Resolution set to fixed
  • Status changed from new to closed

In 25591:

Ignore unauthorized meta keys in meta_form(). fixes #18786.

Note: See TracTickets for help on using tickets.