This critical security issue is being reported publicly per Security Team policy since this module has no stable releases. https://drupal.org/security-advisory-policy

This module exposes 2 menu callbacks that allow nodes to be published and unpublished without confirmation:

<?php
/**
 * Implements hook_menu().
 */
function publish_button_menu() {
 
$items['publish_button/%/publish'] = array(
   
'title' => 'Publish node',
   
'page callback' => 'publish_button_status',
   
'page arguments' => array(1),
   
'type' => MENU_CALLBACK,
   
'access callback' => '_publish_button_menu_access',
   
'access arguments' => array(1, 'publish'),
  );
 
$items['publish_button/%/unpublish'] = array(
   
'title' => 'Unpublish node',
   
'page callback' => 'publish_button_status',
   
'page arguments' => array(1),
   
'type' => MENU_CALLBACK,
   
'access callback' => '_publish_button_menu_access',
   
'access arguments' => array(1, 'unpublish'),
  );
  return
$items;
}
?>

This is an obvious and critical CSRF vulnerability. Either these callbacks should be removed from the module (Other modules like https://drupal.org/project/fasttoggle already provide similar functionality) or the callbacks need to be token protected.

Fasttoggle module provides a token in the link to protect against CSRF:

/toggle/status?destination=node%2F18862&token=faef929bcebeef6672130dc114348a32a

Files: 
CommentFileSizeAuthor
#1 2213403_block_csrf.patch1.48 KBgreggles

Comments

greggles’s picture

Issue summary:View changes
Status:Active» Needs review
StatusFileSize
new1.48 KB

I think this should work. I didn't test it, though.

pwolanin’s picture

Why not just 'token'?

Also, better to check this in the access callback?

greggles’s picture

Two reasons not just 'token.' Frist, it's an overloaded word in our community meaning both token.inc/token.module AND csrf tokens. Second, in the broader world, people often use csrf_token as the name of the token which seems reasonable to me. It helps clarify why you are doing it if someone comes across it and is unsure.

It could be done in the access callback but frequently is not done that way - I don't think there's a good reason why not. @MiSc do you have a preference?

MiSc’s picture

Thanks! Tested, committed and pushed.

MiSc’s picture

Status:Needs review» Fixed
greggles’s picture

Thanks, @MiSc!

I think given the importance of this issue that a stable release would be nice. Can you either make one or comment on #2054049: Create a stable release of publish button module as to what needs to happen first?

Status:Fixed» Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.