Make WordPress Core

Opened 7 years ago

Closed 7 years ago

Last modified 7 years ago

#38293 closed defect (bug) (fixed)

A connected user can delete a protected post meta

Reported by: ajoah's profile ajoah Owned by: johnbillion's profile johnbillion
Milestone: 4.7 Priority: normal
Severity: normal Version: 3.3
Component: Posts, Post Types Keywords: has-patch has-unit-tests commit
Focuses: administration Cc:

Description

Hi,

I discovered that an user can rename (and so delete) a protected post meta if he knows its id.

Example :

My posts meta :
https://snag.gy/RfhUA5.jpg

I send the normal meta name with the protected meta id (24) :

https://snag.gy/SIo9l4.jpg

After update :
https://snag.gy/345HTD.jpg

The problem comes from https://core.trac.wordpress.org/browser/trunk/src/wp-admin/includes/post.php#L291

<?php
        // Meta Stuff
        if ( isset($post_data['meta']) && $post_data['meta'] ) {
                foreach ( $post_data['meta'] as $key => $value ) {
                        if ( !$meta = get_post_meta_by_id( $key ) )
                                continue;
                        if ( $meta->post_id != $post_ID )
                                continue;
                        if ( is_protected_meta( $value['key'], 'post' ) || ! current_user_can( 'edit_post_meta', $post_ID, $value['key'] ) )
                                continue;
                        update_meta( $key, $value['key'], $value['value'] );
                }
        }

The is_protected_meta function is used only on the new name and not on the old name :

$meta->meta_key

Attachments (3)

38293.patch (629 bytes) - added by johnbillion 7 years ago.
38293.diff (2.4 KB) - added by peterwilsoncc 7 years ago.
38293.2.diff (2.0 KB) - added by johnbillion 7 years ago.

Download all attachments as: .zip

Change History (14)

#1 @ajoah
7 years ago

  • Summary changed from An connected user can delete a protected post meta to A connected user can delete a protected post meta

#2 @johnbillion
7 years ago

  • Keywords needs-patch needs-unit-tests added
  • Milestone changed from Awaiting Review to Future Release
  • Version changed from 4.6.1 to 3.3

#3 @johnbillion
7 years ago

Thanks for the report, @ajoah. Confirmed this is a valid bug.

@johnbillion
7 years ago

#4 @johnbillion
7 years ago

  • Keywords has-patch added; needs-patch removed

#5 @johnbillion
7 years ago

Patch. Needs some tests.

@peterwilsoncc
7 years ago

#6 @peterwilsoncc
7 years ago

Unit test added in 38293.diff, it hits the db a few times so is a little slow.

#7 @peterwilsoncc
7 years ago

  • Keywords has-unit-tests added; needs-unit-tests removed

#8 @johnbillion
7 years ago

  • Owner set to johnbillion
  • Status changed from new to reviewing

@johnbillion
7 years ago

#9 @johnbillion
7 years ago

  • Keywords commit added
  • Milestone changed from Future Release to 4.7

Thanks @peterwilsoncc. 38293.2.diff tweaks the test a bit. The condition doesn't rely on a separate user updating the post -- any user cannot alter a protected meta field via edit_post().

#10 @johnbillion
7 years ago

  • Resolution set to fixed
  • Status changed from reviewing to closed

In 39062:

Posts, Post Types: Prevent users from being able to delete a protected meta field from a post.

Previously a user could remove a protected meta field by using their browser developer tools to alter the form field properties in the Custom Fields meta box, given that they know the ID of the protected meta field. This change prevents this by preventing any change to a protected meta field, including changing its key.

Props ajoah, johnbillion, peterwilsoncc
Fixes #38293

#11 @ajoah
7 years ago

Great work, thank you :)

Note: See TracTickets for help on using tickets.