WordPress.org

Make WordPress Core

Opened 9 years ago

Last modified 17 months ago

#10975 new enhancement

comment form nonce

Reported by: tellyworth Owned by:
Milestone: Future Release Priority: normal
Severity: normal Version:
Component: Comments Keywords: needs-patch
Focuses: Cc:

Description

This adds a nonce to public comment forms, via the comment_form action.

Attachments (2)

wp-comment-nonce.patch (1.4 KB) - added by tellyworth 9 years ago.
wp-comment-nonce-refresh.patch (1.3 KB) - added by tellyworth 8 years ago.
refreshed patch tested against trunk

Download all attachments as: .zip

Change History (25)

#1 @scribu
9 years ago

  • Type changed from defect (bug) to enhancement

Can non-logged-in users still comment?

#2 @johnbillion
9 years ago

  • Keywords dev-feedback added

I don't see what this solves. I assume it's attempting to prevent automated commenting by bots. The nonce will be identical for every non-logged in visitor so it'll be very easy for an automated comment bot to get this nonce and use it in its requests, and then we're back to square one.

Scribu: yes they can.

#3 @filosofo
9 years ago

An admin has greater posting privileges than a non-logged-in user, so there would be some advantage just in preventing XSS-commenting for only logged-in users.

And perhaps we could use the client's IP address in the nonce hash when generating it for non-logged-in users?

#4 @tellyworth
9 years ago

"I don't see what this solves. I assume it's attempting to prevent automated commenting by bots. The nonce will be identical for every non-logged in visitor so it'll be very easy for an automated comment bot to get this nonce and use it in its requests, and then we're back to square one."

It blocks several things:

  1. Dumb bots that just drive-by POST without fetching the page. These are still very common, so it will block a substantial amount of spam.
  1. XSS attacks that try to defeat spam filters by tricking real people into submitting an anonymous comment on another blog. Currently rare, but there's no reason to wait until it becomes common.
  1. Slightly less dumb bots that do a single fetch on a blog first, then submit many comments on multiple posts. This won't work with the patch applied because the nonce is unique to each post.

This is not intended to block all spam (that's the job of specialized plugins), just raise the bar a little by eliminating the simplest attacks.

filosofo, incorporating the client's IP address would block some legitimate comments. There is scope for hashing and checking additional info, but I think that's the job of spam filtering plugins, not core (that sort of stuff has to change frequently for it to be effective).

#5 @scribu
9 years ago

  • Milestone changed from Unassigned to 2.9

#6 @ryan
9 years ago

  • Milestone changed from 2.9 to 3.0

#7 @scribu
9 years ago

  • Keywords changed from has-patch, dev-feedback to has-patch dev-feedback

#8 @westi
9 years ago

Nice idea.

I don't think we should use normal nonces for this.

I think it is better that we have a different class of "nonce" for use on the frontend for non-logged in users.

We can then enforce the usage of the current nonces for logged in users only.

#9 @nacin
8 years ago

  • Keywords needs-patch added; has-patch dev-feedback removed

#10 @Denis-de-Bernardy
8 years ago

you can also use wp-hashcash...

in case anyone missed it, such a patch can have severe implications when it comes to caching.

#11 @scribu
8 years ago

  • Milestone changed from 3.0 to Future Release

Punting, pending more discussion.

#12 @ryanhellyer
8 years ago

I just came across this ticket now via a Google search (I need to do something similar for another project).

This seems like something far more suitable for a plugin as I don't see any way of getting around this when caching heavily.

#14 @scribu
8 years ago

Related: #13791

@tellyworth
8 years ago

refreshed patch tested against trunk

#15 @iseulde
5 years ago

  • Component changed from General to Comments

#16 @westonruter
5 years ago

  • Keywords has-patch added; needs-patch removed

#17 @chriscct7
3 years ago

  • Keywords needs-refresh added

This ticket was mentioned in Slack in #core by johnbillion. View the logs.


3 years ago

#19 @johnbillion
3 years ago

  • Keywords needs-patch added; has-patch needs-refresh removed

As per the comments above, this needs to take into consideration persistent caching mechanisms which can cause an out of date nonce to be delivered to a new visitor, preventing them from being able to leave a comment.

As also mentioned in the comments above, this affords no protection for anonymous users. If the nonce was only included and verified for logged in users, then it would solve both issues.

#20 @snarebold
3 years ago

As a result from a professional penetration test

Is it possible to perform CSRF attacks (regarding the comment form) ? Yes, an attacker could e.g. integrate the comment form (HTTP POST) in a hidden iFrame and trick a victim to load this frame. In this way, an attacker could silently post arbitrary comments from the victim's IP address.

#21 @egyptimhotep
17 months ago

This should be in core!

#22 @ocean90
17 months ago

#40043 was marked as a duplicate.

#23 @egyptimhotep
17 months ago

I am sorry for duplicate, but I just want to emphasize the importance of the security.

Note: See TracTickets for help on using tickets.