Make WordPress Core

Opened 15 years ago

Closed 14 years ago

#13791 closed enhancement (duplicate)

Prevent comment author impersonation

Reported by: mdawaffe's profile mdawaffe Owned by:
Milestone: Priority: normal
Severity: normal Version: 2.9.2
Component: Comments Keywords: has-patch
Focuses: Cc:

Description

Currently any logged out commenter can create a comment using a registered user's name and email address. Blog viewers can't tell the difference unless the theme styles the comments differently based on the comment's user_id. Even then, the user_id of a user without the unfiltered_html cap can by spoofed via CSRF.

Whether the comment was submitted by a logged in user is not displayed admin-side either.

To prevent impersonation, the attached:

  1. Extends CSRF protection to cover all logged in commenters, not just the unfiltered comment content of logged in commenters with the unfiltered_html cap.
  2. Fires a new comment_impersonation action during the pre_comment_on_post hook only for logged out users. Attaching it to pre_comment_on_post ensures the new hook does not get fired during imports.
  3. Adds an impersonation detector to that new hook to check pre_comment_author_email for email addresses of registered users.
  4. If impersonation is detected, wp_die()s.

Impersonation of registered users by logged in users is already prevented by wp-comments-post.php (it overwrites the email/name/url submitted by logged in users).

Impersonation of registered users by logged out users is caught by the attached.

Impersonation of unregistered users is fine: let Anonymous have its freedom.

"Impersonation" of registered users by CSRF is prevented by noncing the form for all logged in commenters.

Attachments (2)

13791.diff (5.8 KB) - added by mdawaffe 15 years ago.
13791.2.diff (6.5 KB) - added by mdawaffe 15 years ago.
Removes second nonce from wp_comment_reply()

Download all attachments as: .zip

Change History (11)

@mdawaffe
15 years ago

@mdawaffe
15 years ago

Removes second nonce from wp_comment_reply()

#2 in reply to: ↑ 1 @mdawaffe
15 years ago

Replying to Denis-de-Bernardy:

#8646, #10931

Thanks, I couldn't find anything when I searched.

#3 @mdawaffe
15 years ago

The proposed method in #10931 is less configurable/extendable, but significantly simpler. It also lacks CSRF protection.

#4 follow-up: @filosofo
15 years ago

Impersonation of registered users by logged out users is caught by the attached.

This is really going to annoy people who don't care whether they're logged in or not, but just want to comment on posts.

  • Someone who logs in to make posts, but then replies spontaneously (say using mobile) without logging in--only to lose her comment to a cryptic movie reference ('Howdy, Mr. Abagnale.').
  • Someone who once created an account long ago for some reason, but now returns to make a comment only to lose it upon submission.

Perhaps more importantly, this approach fails at its ostensible purpose, which is to assure readers that a given commenter is who it says it is:

  • If a site requires login for commenting, then this isn't a problem currently.
  • If a site does not require login for commenting, then readers can have no certainty that a given comment hasn't been spoofed, because they likely do not know whether the actual commenter is a registered user. For most sites they don't even have probable confidence, because the vast majority of comments will have been made by those not logged in, which implies that only a minority can possibly be helped by anti-spoofing.

Consider the fact that spoofed comments are a slim minority of comments, and what this approach amounts to is many legitimate users being irritated with only slight confidence that the bad guys are being thwarted. Like airport security procedures? :)

#5 in reply to: ↑ 4 ; follow-up: @mdawaffe
15 years ago

Replying to filosofo:

This is really going to annoy people who don't care whether they're logged in or not, but just want to comment on posts.

  • Someone who logs in to make posts, but then replies spontaneously (say using mobile) without logging in--only to lose her comment to a cryptic movie reference ('Howdy, Mr. Abagnale.').
  • Someone who once created an account long ago for some reason, but now returns to make a comment only to lose it upon submission.

I consider this a Proof of Concept, not a final implementation. A better UX would be to offer a pre-filled comment form (or redirect back to the referrer with the form prefilled), and offer the chance to change the email address or log in.

Perhaps more importantly, this approach fails at its ostensible purpose, which is to assure readers that a given commenter is who it says it is:

  • If a site requires login for commenting, then this isn't a problem currently.

Yes it is. There is a CSRF vulnerability here.

  • If a site does not require login for commenting, then readers can have no certainty that a given comment hasn't been spoofed, because they likely do not know whether the actual commenter is a registered user. For most sites they don't even have probable confidence, because the vast majority of comments will have been made by those not logged in, which implies that only a minority can possibly be helped by anti-spoofing.

It'd be easy to add some style to the comment by a registered user.

Consider the fact that spoofed comments are a slim minority of comments, and what this approach amounts to is many legitimate users being irritated with only slight confidence that the bad guys are being thwarted. Like airport security procedures? :)

See above UX considerations.

#6 in reply to: ↑ 5 @filosofo
15 years ago

Replying to mdawaffe:

Replying to filosofo:
I consider this a Proof of Concept, not a final implementation. A better UX would be to offer a pre-filled comment form (or redirect back to the referrer with the form prefilled), and offer the chance to change the email address or log in.

Something like that would be a necessary part of a spoof check.

Yes it is. There is a CSRF vulnerability here.

I didn't object to the nonce; fixing that is great.

It'd be easy to add some style to the comment by a registered user.

OK, but first, that depends on theme designers to implement. Second, if present that visual cue almost solves the issue by itself: knowing that someone is posting without login gives you reason for greater skepticism of that comment and might encourage users to register for that reason.

#7 @mdawaffe
15 years ago

OK. I agree. The CSRF protection is the most important piece here. How to display or prevent impersonation could be left to a theme or plugin.

#8 @scribu
14 years ago

Related: #10975

#9 @scribu
14 years ago

  • Milestone Awaiting Triage deleted
  • Resolution set to duplicate
  • Status changed from new to closed

I'm going to close this as a duplicate of both #10975 and #10931 so that they can be dealt with separately.

Note: See TracTickets for help on using tickets.