[wp-hackers] (no subject)

[Michael D Adams]

Mark Jaquith's interesting "Rethinking check_admin_referer()" thread  
[1] has generated some POST v. GET debate.  This debate seems to crop  
up once or twice a year, so while people are thinking about it, allow  
me to needlessly fan the flames :)

Of particular interest to me was Bryan Layman's reply [2] suggesting  
that GETs be met with an approval screen and POSTs be checked by  
check_admin_referer() (or whatever security system) before going on  
their merry state changing way.

Though GETs can probably be made just as secure as POSTs, I see no  
harm in changing many of WP's requests to POST.

Here's what I suggest:

1. If non indempotent operations are called by GETs, produce a  
confirmation screen (similar to mailapprovecomment) per [2].

2. If non indempotent operations are called by POSTs, if  
( check_admin_referer() ) go.

3. Change many of WP's admin requests to POSTs.  I may have missed  
some, and some of those listed below are a little silly; I just made  
a cursory glance through wp-admin/.  Some CSS magic would be needed  
in order to keep the admin pages not hideous (this is beyond me).
  a. Delete Post/Page
  b. Delete Comment
  c. (Un)Approve Comment
  d. Delete Category
  e. Delete Link
  f. Move Link
  g. (De)Activate Plugins
  h. Change Themes

4. Add do_action( 'wp_admin_form', what, id ) to all FORMs in the  
admin section.  This allows people to create a plugin that castrates  
check_admin_referer() wherever deemed necessary and to include nonces  
(or anything else) in specific FORMs.


Mark and Owen Winkler proposed a similar idea [3]:
  1. Switch to nonces
  2. Switch to POSTs
  3. if nonce fails, check_admin_referer()
  4. if that fails, present confirmation dialogue.

My proposal is similar, but keeps the referer as the main check and  
allows other checks to be optionally plugged in.  It would not allow  
a complete replacement of check_admin_referer() by nonces, but  
perhaps it's good enough?  Maybe I'm just being lazy and skirting the  
"check_admin_referer() really needs to be rethought" issue.

Michael

[1] http://comox.textdrive.com/pipermail/wp-hackers/2006-April/ 
005666.html
[2] http://comox.textdrive.com/pipermail/wp-hackers/2006-April/ 
005753.html
[3] http://comox.textdrive.com/pipermail/wp-hackers/2006-April/ 
005730.html