From 14416d109eec9d500f7a9add1e9b5ce031ce7c6e Mon Sep 17 00:00:00 2001
From: effulgentsia <effulgentsia@78040.no-reply.drupal.org>
Date: Wed, 23 May 2012 23:28:17 -0500
Subject: [PATCH] refs #1595298 Revert CSRF token checks back to page
 callbacks, not menu access callbacks.

This will be refactored again later when the routing system is upgraded.
---
 core/modules/aggregator/aggregator.admin.inc |  9 +++++++-
 core/modules/aggregator/aggregator.module    | 23 +-------------------
 core/modules/comment/comment.module          | 23 +-------------------
 core/modules/comment/comment.pages.inc       |  9 +++++++-
 core/modules/overlay/overlay.module          | 14 +++++++-----
 5 files changed, 27 insertions(+), 51 deletions(-)

diff --git a/core/modules/aggregator/aggregator.admin.inc b/core/modules/aggregator/aggregator.admin.inc
index ac868b4b452b..d6cc8ecae47b 100644
--- a/core/modules/aggregator/aggregator.admin.inc
+++ b/core/modules/aggregator/aggregator.admin.inc
@@ -401,9 +401,16 @@ function _aggregator_parse_opml($opml) {
  *   An object describing the feed to be refreshed.
  *
  * @see aggregator_menu()
- * @see aggregator_admin_refresh_feed_access()
  */
 function aggregator_admin_refresh_feed($feed) {
+  // @todo CSRF tokens are validated in page callbacks rather than access
+  //   callbacks, because access callbacks are also invoked during menu link
+  //   generation. Add token support to routing: http://drupal.org/node/755584.
+  $token = request()->query->get('token');
+  if (!isset($token) || !drupal_valid_token($token, 'aggregator/update/' . $feed->fid)) {
+    drupal_access_denied();
+  }
+
   aggregator_refresh($feed);
   drupal_goto('admin/config/services/aggregator');
 }
diff --git a/core/modules/aggregator/aggregator.module b/core/modules/aggregator/aggregator.module
index ee285e2538a6..0626f726e3c8 100644
--- a/core/modules/aggregator/aggregator.module
+++ b/core/modules/aggregator/aggregator.module
@@ -138,8 +138,7 @@ function aggregator_menu() {
     'title' => 'Update items',
     'page callback' => 'aggregator_admin_refresh_feed',
     'page arguments' => array(5),
-    'access callback' => 'aggregator_admin_refresh_feed_access',
-    'access arguments' => array(5),
+    'access arguments' => array('administer news feeds'),
     'file' => 'aggregator.admin.inc',
   );
   $items['admin/config/services/aggregator/list'] = array(
@@ -797,23 +796,3 @@ function aggregator_preprocess_block(&$variables) {
     $variables['attributes_array']['role'] = 'complementary';
   }
 }
-
-/**
- * Access callback: Determines if feed refresh is accessible.
- *
- * @param $feed
- *   An object describing the feed to be refreshed.
- *
- * @see aggregator_admin_refresh_feed()
- * @see aggregator_menu()
- */
-function aggregator_admin_refresh_feed_access($feed) {
-  if (!user_access('administer news feeds')) {
-    return FALSE;
-  }
-  $token = request()->query->get('token');
-  if (!isset($token) || !drupal_valid_token($token, 'aggregator/update/' . $feed->fid)) {
-    return FALSE;
-  }
-  return TRUE;
-}
diff --git a/core/modules/comment/comment.module b/core/modules/comment/comment.module
index 97fbdc39a503..58ea4a615500 100644
--- a/core/modules/comment/comment.module
+++ b/core/modules/comment/comment.module
@@ -277,8 +277,7 @@ function comment_menu() {
     'title' => 'Approve',
     'page callback' => 'comment_approve',
     'page arguments' => array(1),
-    'access callback' => 'comment_approve_access',
-    'access arguments' => array(1),
+    'access arguments' => array('administer comments'),
     'file' => 'comment.pages.inc',
     'weight' => 1,
   );
@@ -2515,23 +2514,3 @@ function comment_file_download_access($field, $entity_type, $entity) {
     return FALSE;
   }
 }
-
-/**
- * Access callback: Determines if comment approval is accessible.
- *
- * @param $cid
- *   A comment identifier.
- *
- * @see comment_approve()
- * @see comment_menu()
- */
-function comment_approve_access($cid) {
-  if (!user_access('administer comments')) {
-    return FALSE;
-  }
-  $token = request()->query->get('token');
-  if (!isset($token) || !drupal_valid_token($token, "comment/$cid/approve")) {
-    return FALSE;
-  }
-  return TRUE;
-}
diff --git a/core/modules/comment/comment.pages.inc b/core/modules/comment/comment.pages.inc
index 59423ec7e631..1f00353c69cc 100644
--- a/core/modules/comment/comment.pages.inc
+++ b/core/modules/comment/comment.pages.inc
@@ -105,9 +105,16 @@ function comment_reply(Node $node, $pid = NULL) {
  *   A comment identifier.
  *
  * @see comment_menu()
- * @see comment_approve_access()
  */
 function comment_approve($cid) {
+  // @todo CSRF tokens are validated in page callbacks rather than access
+  //   callbacks, because access callbacks are also invoked during menu link
+  //   generation. Add token support to routing: http://drupal.org/node/755584.
+  $token = request()->query->get('token');
+  if (!isset($token) || !drupal_valid_token($token, "comment/$cid/approve")) {
+    drupal_access_denied();
+  }
+
   if ($comment = comment_load($cid)) {
     $comment->status = COMMENT_PUBLISHED;
     comment_save($comment);
diff --git a/core/modules/overlay/overlay.module b/core/modules/overlay/overlay.module
index 262826012305..5fdd45323bad 100644
--- a/core/modules/overlay/overlay.module
+++ b/core/modules/overlay/overlay.module
@@ -317,11 +317,6 @@ function overlay_user_dismiss_message_access() {
   if (empty($user->uid)) {
     return FALSE;
   }
-  // Protect against cross-site request forgeries by validating a token.
-  $token = request()->query->get('token');
-  if (!isset($token) || !drupal_valid_token($token, 'overlay')) {
-    return FALSE;
-  }
   return TRUE;
 }
 
@@ -333,6 +328,15 @@ function overlay_user_dismiss_message_access() {
  */
 function overlay_user_dismiss_message() {
   global $user;
+
+  // @todo CSRF tokens are validated in page callbacks rather than access
+  //   callbacks, because access callbacks are also invoked during menu link
+  //   generation. Add token support to routing: http://drupal.org/node/755584.
+  $token = request()->query->get('token');
+  if (!isset($token) || !drupal_valid_token($token, 'overlay')) {
+    drupal_access_denied();
+  }
+
   $account = user_load($user->uid);
   $account->data['overlay_message_dismissed'] = 1;
   $account->save();
-- 
GitLab