The help functions will re-run a check if no results are available. For example:

function security_review_check_upload_extensions_help($result = NULL) {
  $element['title'] = t('Allowed upload extensions');
  $element['descriptions'][] = t("The upload module allows users to attach files to content. Some extensions are considered dangerous because the files can be evaluated and then executued in the browser. A malicious user could use this opening to gain control of your site.");
  $last_check = security_review_get_last_check('security_review', 'upload_extensions');
  if ($last_check['skip'] == '1') {
    $element['findings']['descriptions'][] = _security_review_check_skipped($last_check);
  }
  elseif ($last_check['result'] == '0') {
    if (is_null($result)) {
      $result = security_review_check_upload_extensions();
    }
    $element['findings']['descriptions'][] = t('<a href="!link">Alter file upload settings.</a>', array('!link' => url('admin/settings/uploads')));
    $element['findings']['descriptions'][] = t('The following extensions are considered unsafe and should be removed or limited from use. Or, be sure you are not granting untrusted users the ability to upload files.');
    foreach ($result['value'] as $extension) {
      $element['findings']['items'][] = array(
        'raw' => $extension,
        'safe' => check_plain($extension),
      );
    }
  }
  return $element;
}

It calls security_review_check_upload_extensions() if the previous results aren't available in $result, but does not pass $last_check. This variable is set a few lines above and is an argument in all the security check functions: should it be passed here?

Comments

BrockBoland’s picture

BrockBoland commented
Issue summary: View changes

Fixed opening php tag for formatting

aohrvetpv’s picture

aohrvetpv commented

No, $last_check should not be passed in your example. security_review_check_upload_extensions() does not take $last_check as a parameter. The assertion that all check functions take $last_check as a parameter is false. A check function can take $last_check to avoid checking parts of something that has already been checked. For instance, security_review_check_nodes() can use $last_check to avoid rechecking node revisions that have already been checked for unsafe tags and have not changed since they were checked. It does not make sense for security_review_check_upload_extensions() to take $last_check because there is nothing it can avoid rechecking: the allowed extensions may have changed since the last check.

smustgrave’s picture

smustgrave commented
Status: Active » Closed (outdated)

As Drupal6 has been EOL https://www.drupal.org/about/drupal6-eol closing as outdated