While these are not errors for everyone, they are for a large number of us.

  1. Users can register without approval or at least email verification. This is a guaranteed invitation to spammers.
  2. Anonymous Commenting - same as above.
  3. Mollom installed but not protecting any forms.

Comments

nancydru’s picture

/**
 * @file
 * Custom Security Policy checks for the Security Review module.
 */

/**
 * Implements hook_security_checks() on behalf of the User module.
 */
function user_security_checks() {
  $checks = array();

  $checks["user_registration"] = array(
    'title' => "User Registration",
    'callback' => 'edc_policy_security_user_register',
    'module' => 'edc_policy',
    'success' => 'User registration is difficult for spammers.',
    'failure' => 'User registration is wide open to spammers - you WILL get hit.',
    );

  return array('user' => $checks);
}

/**
 * Implements hook_security_checks() on behalf of the Comment module.
 */
function comment_security_checks() {
  $checks = array();

  $checks["anonymous_commenting"] = array(
    'title' => "Anonymous Commenting",
    'callback' => 'edc_policy_security_anon_comment',
    'module' => 'edc_policy',
    'success' => 'Anonymous users may not comment without approval.',
    'failure' => 'Anonymous users may comment without approval - you WILL get spam.',
    );

  return array('comment' => $checks);
}

/**
 * Implements hook_security_checks() on behalf of the Mollom module.
 * This seems to be inactive until the Mollom module is enabled.
 */
function mollom_security_checks() {
  $checks = array();

  $checks["mollom_setup"] = array(
    'title' => "Mollom Setup",
    'callback' => 'mollom_security_setup',
    'module' => 'edc_policy',
    'success' => 'Mollom setup is okay.',
    'failure' => 'Mollom is enabled, but not fully set up.',
    );

  $checks["mollom_test"] = array(
    'title' => "Mollom Test Mode",
    'callback' => 'mollom_security_test',
    'module' => 'edc_policy',
    'success' => 'Mollom is operating in normal mode.',
    'failure' => 'Mollom is enabled, but is operating in test mode.',
    );

  $checks["mollom_forms"] = array(
    'title' => "Mollom Form Protection",
    'callback' => 'mollom_security_forms',
    'module' => 'edc_policy',
    'success' => 'Mollom is protecting some forms.',
    'failure' => 'Mollom is enabled, but is not protecting any forms.',
    );

  return array('mollom' => $checks);
}

/**
 * Security_review callback function.
 */
function mollom_security_forms() {
  $ret = TRUE;
  $check_result_value = array(t('The Mollom module is operating but not protecting any forms.
    You may receive spam postings and users.'));

  $result = db_query('SELECT form_id FROM {mollom_form}')->fetchCol();

  $ret = (count($result) > 0);

  return array('result' => $ret, 'value' => $check_result_value);
}

function mollom_security_forms_help($check = NULL, $skipped_message = NULL) {
  $element = array(
    'title' => t('Mollom Test Mode'),
    'descriptions' => array(),
    'findings' => array(
      'descriptions' => array(),
      'items' => array(),
      ),
    );
  $element['descriptions'][] = t('The Mollom module can "watch" specified forms and take action to minimize spam.
    It is good practice to protect the user registration form if it is open to users registering themselves.
    Protecting any user-submitted forms (e.g. comments) is also considered good practice.');
  $element['descriptions'][] = l(t('Mollom settings'), 'admin/config/content/mollom');

  if (!empty($skipped_message)) {
    $element['findings']['descriptions'][] = $skipped_message;
  }
  else {
    if ($check && $check['result'] == FALSE) {
      foreach ($check['value'] as $value) {
        $element['findings']['items'][] = array('safe' => $value, 'raw' => $value);
      }
    }
    else {
      $result = db_query('SELECT form_id, mode FROM {mollom_form}')->fetchAllKeyed();

      $modes = array(
        MOLLOM_MODE_ANALYSIS => t('Text analysis'),
        MOLLOM_MODE_CAPTCHA => t('CAPTCHA'),
        );

      if ($result) {
        $list = array();
        foreach ($result as $form_id => $mode) {
          $list[] = t('@form_id (@mode)', 
            array('@form_id' => $form_id, '@mode' => $modes[$mode]));
        }
        $element['findings']['descriptions'][] = t('The following forms are subject to Mollom protection:');
        $element['findings']['items'] = $list;
      }
    }
  }

  return $element;
}

/**
 * Security_review callback function.
 */
function mollom_security_test() {
  $ret = TRUE;
  $check_result_value = array(t('The Mollom module is operating on test mode.
    Your spam statistics may not be accurate.'));

  $ret = !variable_get('mollom_testing_mode', 0);

  return array('result' => $ret, 'value' => $check_result_value);
}

function mollom_security_test_help($check = NULL, $skipped_message = NULL) {
  $element = array(
    'title' => t('Mollom Test Mode'),
    'descriptions' => array(),
    'findings' => array(
      'descriptions' => array(),
      'items' => array(),
      ),
    );
  $element['descriptions'][] = t('Submitting "ham", "unsure", or "spam" on a protected form will trigger the
    corresponding behavior, and similarly, word verifications will only respond to "correct" and "incorrect",
    instead of the actual characters asked for.
    This option should be disabled in production environments.');
  $element['descriptions'][] = l(t('Mollom settings'), 'admin/config/content/mollom/settings');

  if (!empty($skipped_message)) {
    $element['findings']['descriptions'][] = $skipped_message;
  }
  else {
    if ($check && $check['result'] == FALSE) {
      foreach ($check['value'] as $value) {
        $element['findings']['items'][] = array('safe' => $value, 'raw' => $value);
      }
    }
    else {
      $roles = user_roles(FALSE, 'administer mollom');
      if ($roles) {
        $element['findings']['descriptions'][] = t('The following roles may aminister Mollom protection:');
        $element['findings']['items'] = $roles;
      }
    }
  }

  return $element;
}

/**
 * Security_review callback function.
 */
function mollom_security_setup() {
  $ret = TRUE;
  $check_result_value = array(t('The Mollom module is available but it is not protecting this site.
    You may get spam.'));

  $status = _mollom_status(FALSE);
  if ($status !== TRUE) {
    // Uh, oh, Mollom is not correctly set up.
    $ret = FALSE;
  }

  return array('result' => $ret, 'value' => $check_result_value);
}

/**
 * Help page for security_review.
 */
function mollom_security_setup_help($check = NULL, $skipped_message = NULL) {
  $element = array(
    'title' => t('Mollom Setup'),
    'descriptions' => array(),
    'findings' => array(
      'descriptions' => array(),
      'items' => array(),
      ),
    );
  $element['descriptions'][] = t('The Mollom module connects to the Mollom anti-spam service.
    It analyzes the text of specified forms or supplies a CAPTCHA challenge to help protect
    your site from spam.');
  $element['descriptions'][] = l(t('Mollom settings'), 'admin/config/content/mollom/settings');

  if (!empty($skipped_message)) {
    $element['findings']['descriptions'][] = $skipped_message;
  }
  else {
    if ($check && $check['result'] == FALSE) {
      foreach ($check['value'] as $value) {
        $element['findings']['items'][] = array('safe' => $value, 'raw' => $value);
      }
    }
    else {
      $roles = user_roles(FALSE, 'bypass mollom protection');
      if ($roles) {
        $element['findings']['descriptions'][] = t('The following roles may bypass Mollom protection:');
        $element['findings']['items'] = $roles;
      }
    }
  }
}

function edc_policy_security_anon_comment() {
  $ret = TRUE;
  $check_result_value = array(t('Anonymous users may comment without approval. You will get spam.'));

  $roles = user_roles(FALSE, 'skip comment approval');
  $ret = !isset($roles[DRUPAL_ANONYMOUS_RID]);

  return array('result' => $ret, 'value' => $check_result_value);
}

function edc_policy_security_user_register() {
  $ret = TRUE;
  $check_result_value = array(t('"Who can register accounts" is set to "Visitors."'));

  $user_register = variable_get('user_register', 1);
  $email_verify = variable_get('user email verification', 1);

  if ($user_register == USER_REGISTER_VISITORS
    && $email_verify == FALSE) {
    // Tsk, tsk, wide open registration.
    $ret = FALSE;
  }

  return array('result' => $ret, 'value' => $check_result_value);
}

function edc_policy_security_user_register_help($check = NULL, $skipped_message = NULL) {
  $element = array(
    'title' => t('User Registration'),
    'descriptions' => array(),
    'findings' => array(
      'descriptions' => array(),
      'items' => array(),
      ),
    );
  $element['descriptions'][] = t('The user registration settings can help to reduce spam attacks.
      Setting "Who can register accounts" to "Administrators only" is the most secure;
      "Visitors, but administrator approval is required " is good, but requires admin action;
      "Visitors" without also using "Require e-mail verification ..." is an open invitation
      to spammers to register and post junk on your site.');
  $element['descriptions'][] = l('User admin settings', 'admin/config/people/accounts');

  if (!empty($skipped_message)) {
    $element['findings']['descriptions'][] = $skipped_message;
  }
  else {
    if ($check && $check['result'] == FALSE) {
    $element['findings']['descriptions'][] = '';
      foreach ($check['value'] as $value) {
        $element['findings']['items'][] = $value;
      }
    }
    else {
      $user_register = variable_get('user_register', 1);
      $email_verify = variable_get('user email verification', 1);

      $register = array(
        USER_REGISTER_ADMINISTRATORS_ONLY => t('Administrators only'),
        USER_REGISTER_VISITORS => t('Visitors'),
        USER_REGISTER_VISITORS_ADMINISTRATIVE_APPROVAL => t('Visitors, but administrator approval is required'),
        );
      $verify = array(t('No'), t('Yes'));

      $element['findings']['items'][] = t('"Who can register accounts" is set to @reg.', array('@reg' => $register[$user_register]));
      $element['findings']['items'][] = t('"Require e-mail verification ..." is set to @ver.', array('@ver' => $verify[$email_verify]));
    }
  }

  return $element;

  return array(
    'title' => "User Registration",
    'descriptions' => array('The user registration settings can help to reduce spam attacks.
      Setting "Who can register accounts" to "Administrators only" is the most secure;
      "Visitors, but administrator approval is required " is good, but requires admin action;
      "Visitors" without also using "Require e-mail verification ..." is an open invitation
      to spammers to register and post junk on your site.',
      l('User admin settings', 'admin/config/people/accounts')),
    'findings' => array(
      'descriptions' => $findings,
      'items' => array(),
      )
    );
}

function edc_policy_security_anon_comment_help($check = NULL, $skipped_message = NULL) {
  $element = array(
    'title' => t('User Registration'),
    'descriptions' => array(),
    'findings' => array(
      'descriptions' => array(),
      'items' => array(),
      ),
    );
  $element['descriptions'][] = t('The "skip comment approval" permission allows users to post comments without going
    through the approval process. If this is available to anonymous users, they will post junk on your site.');
  $element['descriptions'][] = l(t('User permission settings'), 'admin/people/permissions');

  if (!empty($skipped_message)) {
    $element['findings']['descriptions'][] = $skipped_message;
  }
  else {
    if ($check && $check['result'] == FALSE) {
      foreach ($check['value'] as $value) {
        $element['findings']['items'][] = array('safe' => $value, 'raw' => $value);
      }
    }
    else {
      $roles = user_roles(FALSE, 'skip comment approval');
      $element['findings']['descriptions'][] = t('The following roles may post comments without approval:');
      $element['findings']['items'] = $roles;
    }
  }

  return $element;
}
cashwilliams’s picture

Wow, bookmarking this for future use :)

Is this something that should in the security_review module, or would it be a better fit to have something like security_review_extras that contains tests "not for everyone" ?

nancydru’s picture

IMHO, the ones against core should be in an optional security_review_extras module. The ones against Mollom should be in the Mollom module.

generalredneck’s picture

Just a quick followup. Doing a quick google didn't lead me to anything... but has these been added to a "Security Review Extras" module? or they just hangin out here? Since this is all spam related... it might be good as a "SPAM security Review" module as well if noone approves of putting this in Security Review.

nancydru’s picture

I have not submitted an extra module, if that is what you are asking.

generalredneck’s picture

Yeah. that was all I was asking. Thanks.

lomasr’s picture

Assigned: Unassigned » lomasr
smustgrave’s picture

Status: Active » Closed (outdated)

I'm keeping an eye on the 7.x branch of this module, reviews and majors, but
active work is going toward 2.x (supporting D10)

If any of these are valid for 2.x please open a ticket for them.