Make WordPress Core

Opened 11 years ago

Closed 9 years ago

Last modified 9 years ago

#24030 closed defect (bug) (fixed)

Add an action for when nonce verification fails

Reported by: dd32's profile dd32 Owned by: shelob9's profile Shelob9
Milestone: 4.4 Priority: normal
Severity: normal Version: 2.0.3
Component: Security Keywords: has-patch 4.4-early
Focuses: Cc:

Description

Currently when a plugin (or core) calls check_admin_referer() there is no way for auditing (or debugging) plugins to hook in and record an event that the nonce check failed.

Previously it was possible to use the explain_nonce_$nonce filter to do this, but that was removed in [21133].

If a plugin wants to record an event for a failing nonce, it'll need to call wp_verify_nonce() manually itself, and die afterwards, or call check_admin_referer() after verifying the nonce itself for logging purposes.

I'd suggest either resurrecting the previous filter as an action (for back compat) or adding a new nonce failure hook.

Attachments (4)

24030.diff (853 bytes) - added by Shelob9 10 years ago.
24030-1.patch (1.0 KB) - added by garza 10 years ago.
wp_verify_nonce_test.php (1.4 KB) - added by garza 10 years ago.
unit test for patch 24030-1
24030.2.diff (1.9 KB) - added by johnbillion 9 years ago.

Download all attachments as: .zip

Change History (15)

#1 @dd32
10 years ago

#21190 was marked as a duplicate.

#2 @nacin
10 years ago

  • Component changed from General to Security
  • Keywords needs-patch good-first-bug added
  • Milestone changed from Awaiting Review to Future Release

#3 @Shelob9
10 years ago

  • Keywords has-patch added; needs-patch removed

I'm presuming that the very complex filter that was removed in 21133 was removed for its complexity, so I have created a very simple action. I used it successfully to log actions that failed because the nonce wasn't correct. My specific test was copying the link for switching the theme to Twenty Eleven, and removing the last character from the nonce string before submitting it. I was able to log 'switch-theme_twentyeleven' using the action added in my patch 24030.diff

If something more complicated is needed, please let me know and I will take a stab at it.

Last edited 10 years ago by Shelob9 (previous) (diff)

@Shelob9
10 years ago

#4 @SergeyBiryukov
10 years ago

  • Milestone changed from Future Release to 3.9

#5 @DrewAPicture
10 years ago

  • Owner set to Shelob9
  • Status changed from new to assigned

#6 @johnbillion
10 years ago

We should add the equivalent to check_ajax_referer() too.

#7 @nacin
10 years ago

  • Milestone changed from 3.9 to Future Release

There are existing hooks in check_(admin|ajax)_referer, so we should be able to just append _failed to those to get the other hook name.

Or, should this be done directly in wp_verify_nonce()?

@garza
10 years ago

@garza
10 years ago

unit test for patch 24030-1

#8 @garza
10 years ago

  • Keywords dev-feedback needs-testing added

Uploaded a new patch that moves the action created by Shelob9 into wp_verify_nonce() per the last comment by nacin.

Also created a unit test case which makes sure this action is getting called and passing the $uid and $action

@johnbillion
9 years ago

#9 @johnbillion
9 years ago

  • Keywords 4.4-early added; good-first-bug dev-feedback needs-testing removed
  • Version changed from 3.4.1 to 2.0.3

I recently had a need for this.

24030.2.diff refreshes and tidies up the patch and tests from @garza.

Related: #33342 #32207

Last edited 9 years ago by johnbillion (previous) (diff)

#10 @SergeyBiryukov
9 years ago

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

In 33744:

Add 'wp_verify_nonce_failed' action that fires when nonce verification fails.

props johnbillion, garza, Shelob9.
fixes #24030.

#11 @SergeyBiryukov
9 years ago

  • Milestone changed from Future Release to 4.4
Note: See TracTickets for help on using tickets.