WordPress.org

Make WordPress Core

Opened 20 months ago

Closed 19 months ago

Last modified 19 months ago

#38293 closed defect (bug) (fixed)

A connected user can delete a protected post meta

Reported by: ajoah Owned by: 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 19 months ago.
38293.diff (2.4 KB) - added by peterwilsoncc 19 months ago.
38293.2.diff (2.0 KB) - added by johnbillion 19 months ago.

Download all attachments as: .zip

Change History (14)

#1 @ajoah
20 months 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
19 months 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
19 months ago

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

@johnbillion
19 months ago

#4 @johnbillion
19 months ago

  • Keywords has-patch added; needs-patch removed

#5 @johnbillion
19 months ago

Patch. Needs some tests.

#6 @peterwilsoncc
19 months ago

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

#7 @peterwilsoncc
19 months ago

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

#8 @johnbillion
19 months ago

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

#9 @johnbillion
19 months 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
19 months 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
19 months ago

Great work, thank you :)

Note: See TracTickets for help on using tickets.