[wp-hackers] Rethinking check_admin_referer()

[Owen Winkler]

> 1) The problems present in the current system.

There are many layers to this problem, which I think is what might make 
it so confusing.

Here is the current system:

A malefactor creates a link somewhere whose URL would normally be used 
by your blog's WordPress to trigger the deletion of a post with a 
specific ID (or delete some other indexed item).  The expectation is 
that you will have already logged in to your blog, so when you visit 
that page, you will not have the benefit of the login page as a barrier 
to the immediate deletion of that post.

The current system requires that a referer be sent to visit any admin 
page (although there are some that are not covered, either by choice or 
omission), and that referer must contain a URL from inside your WP admin 
panel.

This prevents the scenario fairly well, since you must now already be on 
an admin page to activate a link that deletes anything.  However, there 
are some issues with this behavior:

i) User Experience - Some browsers, some proxies, and some firewall 
software explicitly block or remove the referer information from the 
request.  As a result, admin pages do not work properly and provide a 
poor error message.  This issue has resulted in numerous support 
requests, for example:

http://wordpress.org/support/topic/15273
http://wordpress.org/support/topic/12102
http://wordpress.org/support/topic/64566
http://wordpress.org/support/topic/5573

The last thread demonstrates that this issue has been ongoing for almost 
a year.  There is also a Codex page dedicated to working around this 
problem, possibly implying that it is a common enough complaint to 
warrant documentation.

ii) Security - The system is not impenetrable.  If an unfiltered URL to 
an admin page that deletes things appears /within/ the admin (such as a 
link in a comment from the comment moderation page), and the admin 
inadvertently clicks on it, it will trigger the deletion.


> 2) A list of the options.

a) Leave things as is - This option suggests that the small amount of 
users affected by this issue are not worth the trouble of any fix.

b) Replace the referer check with nonces - Instead of verifying the 
referer at all, forms and links would be modified to include a special 
code (a nonce) that would be checked before an action is performed. 
Without the presence of this code, the action could either fail with an 
error (similar to the current referer check error) or ask "Are You 
Sure?" to confirm the action, submitting the original data on to the 
action page with an added valid nonce.

Nonces could be generated in these ways:

b-1) One method would simply generate a random value and store it in the 
database.  When verifying a nonce, WP would compare the submitted nonce 
to the one in the database.

b-2) One method would algorithmically generate a nonce based on user, 
action, time, and possibly other variables.  This nonce would not be 
stored on the server, but could be verified algorithmically.

c) Augment the referer check with nonces - Similar to option b, nonces 
would be added to forms and links when possible.  To verify a page 
request, first the nonce would be checked.  If it fails, then the 
referer would be checked as currently done.  If the referer check fails, 
then an "Are You Sure?" confirmation would appear.  If any of the checks 
pass, then no confirmation is displayed and the action continues as normal.

This option could also be pluggable, where a user can control which 
method to use by activating plugins.  A plugin could also deactivate any 
intrinsic behavior.

d) Convert every admin action that works via a GET request to a POST 
request.  GET requests would be made to work only after confirmations. 
POST requests would use the existing admin referer check.

> 3) Their relative pros/cons.

a) No new code must be written or supported.

Support requests will likely continue, and the documentation on enabling 
referer checks should likely need to be reviewed and enhanced.  Those 
browsers, proxies, and firewalls that disable access to WordPress' admin 
will likely continue to do so.  This entire topic of discussion will 
likely have been a gigantic waste of time and energy, and we will 
probably end up arguing about this again at a later time.  ;)

b) We will pick up support for users who currently have trouble with the 
referer issue.  Support for referers should be reduced, since nothing 
needs to be done to "activate" nonces.

Replacing the existing system will require new code, which must be 
tested.  Most of the code can be centralized (generating and checking 
the nonces) but each admin page will most likely be touched.  There are 
some questions about plugin admin pages that still need to be addressed, 
such as how legacy plugins will handle incoming requests since they will 
not already have the nonce generating code.

b-1) Provides a higher level of security.

Requires database storage for nonces, increasing database thrash while 
operating the admin.  May need to store more than one nonce per user 
(ie.- +per action, +per IP).  Depending on how well the data storage 
system works, this option might cause issues with using multiple browser 
instances of the admin panel or the back button.  This could happen 
when, for example, a user clicks the back button to a form that contains 
an old, previously used nonce, then submits that form.

b-2) Does not provide absolute security, since nonces generated 
algorithmically within a time period are likely to be similar or identical.

Does not require database storage.  Nonces could be generated that are 
specific to user, action, and time that do not fail after using the back 
button or using the admin from two open browsers.

c) Covers all options of b and provides some backwards compatibility for 
admin pages (via core or plugin) that do not yet have the nonces 
embedded in their links and forms.

Has all of the cons of b.

d) Puts an end to the discussion of whether actions should be performed 
on a GET request.  Makes it easier to add nonces to the forms, since all 
actions will be forms using POST requests.

Requires that a volume of links must be changed into forms, and that CSS 
be applied to prevent the admin from turning into an ugly spectacle.  By 
itself it doesn't really address the presented issues, but might better 
enable the application of b or c.

> 4) Your opinion.

If we do anything at all, it should be c with b-2.  Option c provides an 
immediate benefit to user experience by replacing most referer checks 
with nonces.  In the case that checks fail, users will no longer see an 
uninformative and prohibitive die() message, but a confirmation that 
allows them to continue.

Allowing the system to be pluggable allows people who are more concerned 
with security than what b-2 provides to use a plugin to augment that 
security.  I am of the opinion that the case is very rare that I (or 
anyone) would even have the opportunity to click on a malicious comment 
link in my admin panel, much less actually do it.  I don't believe that 
we should waste our efforts writing code for a problem that I'm not even 
aware of being in the wild, but allowing for someone else to plug the 
hole seems wise.

On the other hand, even I have been affected by the referer issue, and 
eliminating this support issue would leave us open to deal with more 
important things.

The very, very least we could do is already done - making 
check_admin_referers() pluggable.  You can write a plugin today that 
replaces that function with something that returns true no matter what. 
  It's not secure, but it'll enable the UI for people using those 
browsers, proxies, and firewalls.  This could be improved by making that 
plugin hook in that function able to circumvent the actual check.  Any 
other changes are going to be more invasive of the admin panel.

Does that cover it?

Owen