I noticed this bit of code in drupal_settings_initialize():

    // We escape the hostname because it can be modified by a visitor.
    if (!empty($_SERVER['HTTP_HOST'])) {
      $cookie_domain = check_plain($_SERVER['HTTP_HOST']);
    }

However, AFAIK that's already done earlier in the bootstrap by drupal_environment_initialize():

  if (isset($_SERVER['HTTP_HOST'])) {
    // As HTTP_HOST is user input, ensure it only contains characters allowed
    // in hostnames. See RFC 952 (and RFC 2181).
    // $_SERVER['HTTP_HOST'] is lowercased here per specifications.
    $_SERVER['HTTP_HOST'] = strtolower($_SERVER['HTTP_HOST']);
    if (!drupal_valid_http_host($_SERVER['HTTP_HOST'])) {
      // HTTP_HOST is invalid, e.g. if containing slashes it may be an attack.
      header($_SERVER['SERVER_PROTOCOL'] . ' 400 Bad Request');
      exit;
    }
  }
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Damien Tournoud’s picture

Status: Active » Needs review
FileSize
997 bytes

Agreed. The check_plain() call was a last-resort measure, but we now have a proper sanitizing for this field.

agentrickard’s picture

Status: Needs review » Reviewed & tested by the community

Confirmed that the operation order is correct. Patch passes review, too.

webchick’s picture

Status: Reviewed & tested by the community » Fixed

Ah, fun in a debugger. :)

Committed to HEAD.

Status: Fixed » Closed (fixed)

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