Security checking in meta_box save is reluctant?

1 Views

For example, in this tutorial, the code suggested when saving meta box data is 

/* Verify the nonce before proceeding. */
if ( !isset( $_POST['smashing_post_class_nonce'] ) || !wp_verify_nonce( $_POST['smashing_post_class_nonce'], basename( __FILE__ ) ) )
    return $post_id;

/* Get the post type object. */
$post_type = get_post_type_object( $post->post_type );

/* Check if the current user has permission to edit the post. */
if ( !current_user_can( $post_type->cap->edit_post, $post_id ) )
    return $post_id;

The checking of nonce and user role seems reluctant, seems they hook into the save_post, which is almost the last line in the function wp_insert_post, WordPress should already performed the needed checks, right?

Read More

So I can safely remove them?

Post Views: 1

Related posts

1 comment

  1. No, not really. You’re assuming the function that called wp_insert_post() has already performed those checks. But wp_insert_post() may be used in other pages too, not just the edit page, by plugins, or even themes (many of them with security holes).

    That’s why you should ensure that your code runs only where you want to by using the nonce.

    Ok I’m editing the answer because the comment reply would be too long.

    If a plugin expose wp_insert_post, it is not only my new
    custom field will be hacked, it is all the post fields will get
    hacked. (again, this is the bug in plugin) So, I really can’t see
    verifying my own custom field can bring any additional security

    But it would get “hacked” with the help of your code. Think about the quick post form on dashboard home. It doesn’t have input fields for your post meta, does it? On submission, wp_insert_post -> save_post gets called and if $_POST contains your meta fields (it’s easy to edit the source and add some), they will get saved by your meta box code. Essentially, if you don’t do nonce checks, your making it possible to change your meta fields from any form in WordPress that involves post editing, not just the one to which you attached your custom input fields. Is this what you want?

    I am NOT creating a new action, so if the core wordpress’s code allow
    calling wp_insert_post without verifying the nonce, this is a bug in
    WP. (As you can see they all called the check_admin_referer in the
    post.php.

    The purpose of wp_insert_post is to insert posts, not to check from where the data comes from. However I do agree that the save_post action was badly placed. Such functions should not fire actions and filters at all. Instead the action should have been fired by each form submission handler. Besides the fact that this action introduces more inconsistencies in the WP API, it also facilitates exploits…

Comments are closed.