Ticket #5838 (closed defect: fixed)

Opened 3 months ago

Last modified 3 months ago

Make Nonce Mismatch Fail Instead of AYS

Reported by: filosofo Assigned to: anonymous
Priority: normal Milestone: 2.3.4
Component: Security Version: 2.3.3
Severity: normal Keywords: nonce ays csrf css security has-patch
Cc: filosofo

Description

As the post here points out (I've duplicated his attack using my own 2.3.3 setup), you can make a CSRF attack that tricks a WordPress user into changing the admin password and emailing it to someone, by hiding all of the nonce confirmation except the "yes" submit button.

When the nonce doesn't match, my patch lets you know that the action has failed, and it provides a link back to the referring page so that you can try again.

Attachments

nonce_fail.diff (10.0 kB) - added by filosofo on 02/13/08 16:18:00.
nonce_failure.jpg (16.6 kB) - added by filosofo on 02/13/08 16:18:34.
Screenshot of new nonce mismatch failure message
233_nonce_fix.diff (9.8 kB) - added by filosofo on 02/13/08 18:01:22.

Change History

02/13/08 16:18:00 changed by filosofo

  • attachment nonce_fail.diff added.

02/13/08 16:18:34 changed by filosofo

  • attachment nonce_failure.jpg added.

Screenshot of new nonce mismatch failure message

02/13/08 17:39:41 changed by ryan

  • status changed from new to closed.
  • resolution set to fixed.

(In [6813]) Make Nonce Mismatch Fail Instead of AYS. Props filosofo. fixes #5838 for 2.5

02/13/08 17:41:01 changed by ryan

  • status changed from closed to reopened.
  • resolution deleted.
  • milestone changed from 2.5 to 2.3.4.

(follow-up: ↓ 4 ) 02/13/08 17:41:22 changed by ryan

Anyone want to port that to 2.3?

(in reply to: ↑ 3 ) 02/13/08 17:57:31 changed by filosofo

Replying to ryan:

Anyone want to port that to 2.3?

I've uploaded a patch for 2.3.3 tested and working on my install.

02/13/08 18:01:22 changed by filosofo

  • attachment 233_nonce_fix.diff added.

02/13/08 18:01:51 changed by mdawaffe

A possible alternative would be to still show the AYS screen, not update the nonce, but require the user to enter his or her password (or username and password) to authenticate the request.

Pros: keeps the insurance we already have of not losing a POST request because of failed/expired nonce. That's a rare event. I think every time I've gotten an AYS it was because of bad code, not an expired nonce, so resubmitting by clicking yes didn't work anyway.

Cons: harder to code, needs to be audited, breaks the "deny, don't fix" security philosophy, is still open to social engineering CSRF if a user uses the same username/password pair on many sites. Such a CSRF attack could use a form like the following using similar techniques to the one above. 

 Sign up for the new hotness!
 username ____
 password ____

... submit... Attack successful: those fields were actually the nonce AYS username/password fields. The user put their "tried and true" username/password pair into them.

Unrelatedly, a plugin can be written to fix this security issue. Just replace check_admin_referer() (it's pluggable).

02/13/08 18:14:39 changed by ryan

(In [6817]) Add a prophylactic specialchars to the object in explain nonce. see #5838

02/13/08 18:16:02 changed by ryan

I'm fine with just die-ing. wp_nonce_ays() has caused enough pain. :-)

02/13/08 18:17:47 changed by ryan

  • status changed from reopened to closed.
  • resolution set to fixed.

(In [6818]) Make Nonce Mismatch Fail Instead of AYS. Props filosofo. fixes #5838 for 2.3