WordPress.org

Make WordPress Core

Opened 4 years ago

Closed 4 years ago

Last modified 4 years ago

#11810 closed defect (bug) (fixed)

Some users able to comment on unpublished posts

Reported by: ericmann Owned by:
Milestone: 2.9.2 Priority: normal
Severity: normal Version: 2.9.1
Component: Comments Keywords: has-patch needs-testing
Focuses: Cc:

Description

This was originally reported on the WP support forums.

Users with certain developer tools (i.e Firebug) can manually edit the comment_post_ID field of the default commentform and submit a comment to any post on the site, whether it's published or not (or closed to comments or not).

Perhaps we should consider some level of security for comments to ensure this can't happen? Maybe hash the comment_post_ID field so it can't be edited in plaintext?

Attachments (1)

no-comments-on-non-accessible-posts.11810.diff (879 bytes) - added by filosofo 4 years ago.

Download all attachments as: .zip

Change History (22)

comment:1 follow-up: ericmann4 years ago

  • Summary changed from Some users able to comment on closed posts to Some users able to comment on unpublished posts

Update: Apparently closed-comment posts remain closed. I can't reproduce the other users's exact error. However, you can still force a comment on an unpublished post.

comment:2 in reply to: ↑ 1 ericmann4 years ago

Replying to ericmann:

Update: Apparently closed-comment posts remain closed. I can't reproduce the other users's exact error. However, you can still force a comment on an unpublished post.

Also allows sending comments to password-protected posts. Private posts are unaffected.

comment:3 filosofo4 years ago

Could you explain how to reproduce the error?

The following lines seem to preclude posting on unpublished or closed posts:

 27 } elseif ( !comments_open($comment_post_ID) ) {                                                                   
 28         do_action('comment_closed', $comment_post_ID);
 29         wp_die( __('Sorry, comments are closed for this item.') );                                                
 30 } elseif ( in_array($status->post_status, array('draft', 'pending') ) ) { 
 31         do_action('comment_on_draft', $comment_post_ID); 
 32         exit;

I'm probably missing something obvious, but aren't private posts and password-protected posts the same?

comment:4 filosofo4 years ago

Sorry, need more coffee: those lines are in wp-comments-post.php

comment:5 filosofo4 years ago

And please ignore the last comment about private and password-protected posts. Duh. Really need the coffee...

comment:6 ericmann4 years ago

I tested the following post types:
Published
Scheduled
Draft
Private
Password Protected

If you load the comments form in Firefox, you can manually edit the hidden field 'comment_post_ID' and input any post ID you want. If the post ID you're entering belongs to a published, scheduled, or password protected post, your comment goes through.

If the ID belongs to a draft or private post, though, you get an error.

comment:7 nacin4 years ago

Okay, sounds like we need to do check post_password_required() and current_user_can('read_post', id)?

comment:8 follow-up: filosofo4 years ago

Patch attached, but not using current_user_can() check, because it returns false for non-logged-in users.

Since we don't allow comments on "pending" despite capability, there's no reason to allow them on "future," right?

comment:9 ericmann4 years ago

  • Keywords has-patch needs-testing added

The logic in your patch looks good. I'll give it a run and see how things work.

comment:10 follow-up: nacin4 years ago

Replying to filosofo:

Patch attached, but not using current_user_can() check, because it returns false for non-logged-in users.

True, but we still need to cover our bases for a private post.

Since we don't allow comments on "pending" despite capability, there's no reason to allow them on "future," right?

Hypothetically, pending status is a type of draft status, while future is a form of a published post. I don't think there are other restrictions (in wp-comments-post, admin-ajax, the comments template, etc.) on commenting on a future post just as long as they have capabilities to see the post.

comment:11 in reply to: ↑ 8 ; follow-up: ericmann4 years ago

Replying to filosofo:

Patch attached, but not using current_user_can() check, because it returns false for non-logged-in users.

Since we don't allow comments on "pending" despite capability, there's no reason to allow them on "future," right?

So your patch keeps a not-logged-in user from creating comments for all the different kinds of posts, but rather than supply a useful error, it just dumps some to a blank page. For example, if you try posting a comment to a password protected or future post, you are dumped to a blank page with no branding, no content, and no explanation as to why.

It also doesn't prevent users from posting to other posts (which wasn't addressed in the original ticket). But I can comment on post ID 130 from post ID 1 if both posts are published, public, and open to comments.

comment:12 in reply to: ↑ 10 filosofo4 years ago

Replying to nacin:

True, but we still need to cover our bases for a private post.

I thought private posts were already covered?

Hypothetically, pending status is a type of draft status, while future is a form of a published post. I don't think there are other restrictions (in wp-comments-post, admin-ajax, the comments template, etc.) on commenting on a future post just as long as they have capabilities to see the post.

Currently you have to be able to edit a particular future post in order to view it (in WP_Query); it seems to me that more reasonable check for allowing a comment on a future post would be "read_post," but that would involve changing the "read_post" logic. This gets complicated...

comment:13 in reply to: ↑ 11 ; follow-up: filosofo4 years ago

Replying to ericmann:

So your patch keeps a not-logged-in user from creating comments for all the different kinds of posts

No. Why do you say that?

For example, if you try posting a comment to a password protected or future post, you are dumped to a blank page with no branding, no content, and no explanation as to why.

That's current behavior for commenting on drafts, pending, or trashed posts.

It also doesn't prevent users from posting to other posts (which wasn't addressed in the original ticket). But I can comment on post ID 130 from post ID 1 if both posts are published, public, and open to comments.

I don't see why that's a bug, except perhaps in HTTP itself.

comment:14 in reply to: ↑ 13 ; follow-ups: ericmann4 years ago

Replying to filosofo:

Replying to ericmann:

So your patch keeps a not-logged-in user from creating comments for all the different kinds of posts

No. Why do you say that?

I meant it worked for the different kinds of posts we're working on. Not-logged-in users cannot comment on private, password protected, draft, or future posts. This is what we want.

For example, if you try posting a comment to a password protected or future post, you are dumped to a blank page with no branding, no content, and no explanation as to why.

That's current behavior for commenting on drafts, pending, or trashed posts.

But is that the behavior we want?

It also doesn't prevent users from posting to other posts (which wasn't addressed in the original ticket). But I can comment on post ID 130 from post ID 1 if both posts are published, public, and open to comments.

I don't see why that's a bug, except perhaps in HTTP itself.

It's not exactly a bug, but could be a problem. Logically, you should only be able to comment on the post you're on ... not any post you want by changing the post ID.

comment:15 in reply to: ↑ 14 ; follow-up: nacin4 years ago

For example, if you try posting a comment to a password protected or future post, you are dumped to a blank page with no branding, no content, and no explanation as to why.

That's current behavior for commenting on drafts, pending, or trashed posts.

But is that the behavior we want?

Outside the scope of this ticket, I think. There are other tickets that discuss this. Basically, it is difficult to offer a branded warning as it requires the theme to cooperate. This was also in the context of, say, a warning for if the comment was blank. If you're trying to post to something for which you don't have permissions and have to modify what is posted to do so, you deserve a wp_die().

comment:16 in reply to: ↑ 15 ericmann4 years ago

Replying to nacin:

Outside the scope of this ticket, I think. There are other tickets that discuss this. Basically, it is difficult to offer a branded warning as it requires the theme to cooperate. This was also in the context of, say, a warning for if the comment was blank. If you're trying to post to something for which you don't have permissions and have to modify what is posted to do so, you deserve a wp_die().

wp_die() is appropriate in this case. In fact, that's what you get when you try to post to comment on a Private post.

comment:17 scribu4 years ago

Related: #11695

comment:18 in reply to: ↑ 14 filosofo4 years ago

Replying to ericmann:

It's not exactly a bug, but could be a problem. Logically, you should only be able to comment on the post you're on ... not any post you want by changing the post ID.

The problem is defining "post you're on" in a meaningful way, since HTTP is stateless.

comment:19 ryan4 years ago

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

(In [12647]) Block comments for future posts and password protected posts (when password not provided). Props filosofo. fixes #11810 for trunk

comment:20 ryan4 years ago

(In [12649]) Block comments for future posts and password protected posts (when password not provided). Props filosofo. fixes #11810 for 2.8

comment:21 ryan4 years ago

[12648] for 2.9 branch.

Note: See TracTickets for help on using tickets.