WordPress.org

Make WordPress Core

Opened 10 years ago

Last modified 8 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:
PR Number:

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 10 years ago.
wp-comment-nonce-refresh.patch (1.3 KB) - added by tellyworth 9 years ago.
refreshed patch tested against trunk

Download all attachments as: .zip

Change History (26)

#1 @scribu
10 years ago

  • Type changed from defect (bug) to enhancement

Can non-logged-in users still comment?

#2 @johnbillion
10 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
10 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
10 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
10 years ago

  • Milestone changed from Unassigned to 2.9

#6 @ryan
10 years ago

  • Milestone changed from 2.9 to 3.0

#7 @scribu
10 years ago

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

#8 @westi
10 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
10 years ago

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

#10 @Denis-de-Bernardy
10 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
10 years ago

  • Milestone changed from 3.0 to Future Release

Punting, pending more discussion.

#12 @ryanhellyer
9 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
9 years ago

Related: #13791

@tellyworth
9 years ago

refreshed patch tested against trunk

#15 @iseulde
6 years ago

  • Component changed from General to Comments

#16 @westonruter
6 years ago

  • Keywords has-patch added; needs-patch removed

#17 @chriscct7
4 years ago

  • Keywords needs-refresh added

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


4 years ago

#19 @johnbillion
4 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
4 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
3 years ago

This should be in core!

#22 @ocean90
3 years ago

#40043 was marked as a duplicate.

#23 @egyptimhotep
3 years ago

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

#24 @ayeshrajans
8 months ago

Hello everyone,
I'm sorry for bringing years old issues back. I have created a new plugin https://wordpress.org/plugins/comment-form-csrf-protection/ for those who really care about this issue. I'd be happy to work on a core patch if it helps.

Note: See TracTickets for help on using tickets.