? filter_xss_bad_protocol-fix-715142-28.patch
? sites/default/files
? sites/default/private
? sites/default/settings.php
Index: update.php
===================================================================
RCS file: /cvs/drupal/drupal/update.php,v
retrieving revision 1.319
diff -u -p -r1.319 update.php
--- update.php	11 Apr 2010 18:33:43 -0000	1.319
+++ update.php	23 Apr 2010 16:47:34 -0000
@@ -317,7 +317,7 @@ function update_check_requirements() {
     update_task_list('requirements');
     drupal_set_title('Requirements problem');
     $status_report = theme('status_report', array('requirements' => $requirements));
-    $status_report .= 'Check the error messages and <a href="' . check_url(request_uri()) . '">try again</a>.';
+    $status_report .= 'Check the error messages and <a href="' . check_plain(request_uri()) . '">try again</a>.';
     print theme('update_page', array('content' => $status_report));
     exit();
   }
Index: includes/common.inc
===================================================================
RCS file: /cvs/drupal/drupal/includes/common.inc,v
retrieving revision 1.1148
diff -u -p -r1.1148 common.inc
--- includes/common.inc	22 Apr 2010 08:18:56 -0000	1.1148
+++ includes/common.inc	23 Apr 2010 16:47:35 -0000
@@ -1181,10 +1181,21 @@ function flood_is_allowed($name, $thresh
  */
 
 /**
- * Prepare a URL for use in an HTML attribute. Strips harmful protocols.
- */
-function check_url($uri) {
-  return filter_xss_bad_protocol($uri, FALSE);
+ * Strip harmful protocols (e.g., 'javascript:') from a URL and optionally make it safe for output to HTML.
+ *
+ * @param $uri
+ *   A plain-text URI that might contain harmful protocols.
+ * @param $check_plain
+ *   Whether to also call check_plain() so that the result can be safely output
+ *   to HTML. Defaults to TRUE, but can be set to FALSE when the result of this
+ *   function will not be output to HTML (e.g., to a plain-text email instead),
+ *   or when it will be passed to another function that expects a plain-text
+ *   string instead of an HTML-encoded string (e.g., drupal_attributes(), t(),
+ *   or l()).
+ */
+function check_url($uri, $check_plain = TRUE) {
+  $uri = filter_xss_bad_protocol($uri, FALSE);
+  return $check_plain ? check_plain($uri) : $uri;
 }
 
 /**
@@ -1422,25 +1433,36 @@ function _filter_xss_attributes($attr) {
 }
 
 /**
- * Processes an HTML attribute value and ensures it does not contain an URL with a disallowed protocol (e.g. javascript:).
+ * Processes a text or HTML string to ensure it does not contain a URL with a disallowed protocol (e.g. javascript:).
+ *
+ * This function is usually called as part of some other sanitization process
+ * (e.g., check_url(), filter_xss(), or check_markup()) rather than on its own.
+ * Most code should call one of those functions instead. One exception is that
+ * url() and url_is_external() call this function directly, not as part of
+ * sanitization, but in order to determine if a URL is external, and not wanting
+ * to treat unknown protocols as external.
  *
  * @param $string
- *   The string with the attribute value.
- * @param $decode
- *   Whether to decode entities in the $string. Set to FALSE if the $string
- *   is in plain text, TRUE otherwise. Defaults to TRUE.
+ *   The string that contains one or more URLs within it. Can either be a plain
+ *   text string or be encoded for HTML.
+ * @param $html_encoded
+ *   Whether $string is already encoded for HTML (for example, if it was parsed
+ *   from an HTML document or an HTML editor). If TRUE, the returned string will
+ *   also be escaped for HTML. If FALSE, the returned string will not be
+ *   HTML-escaped, so check_plain() will need to be called prior to outputting
+ *   to a Drupal page. Defaults to TRUE.
  * @return
- *   Cleaned up and HTML-escaped version of $string.
+ *   Cleaned up version of $string.
  */
-function filter_xss_bad_protocol($string, $decode = TRUE) {
+function filter_xss_bad_protocol($string, $html_encoded = TRUE) {
   static $allowed_protocols;
 
   if (!isset($allowed_protocols)) {
     $allowed_protocols = array_flip(variable_get('filter_allowed_protocols', array('ftp', 'http', 'https', 'irc', 'mailto', 'news', 'nntp', 'rtsp', 'sftp', 'ssh', 'telnet', 'webcal')));
   }
 
-  // Get the plain text representation of the attribute value (i.e. its meaning).
-  if ($decode) {
+  // If the string is HTML-encoded, decode it.
+  if ($html_encoded) {
     $string = decode_entities($string);
   }
 
@@ -1465,7 +1487,12 @@ function filter_xss_bad_protocol($string
     }
   } while ($before != $string);
 
-  return check_plain($string);
+  // If the string was HTML-encoded and we decoded it, re-encode it.
+  if ($html_encoded) {
+    $string = check_plain($string);
+  }
+
+  return $string;
 }
 
 /**
@@ -1489,7 +1516,7 @@ function format_rss_channel($title, $lin
 
   $output = "<channel>\n";
   $output .= ' <title>' . check_plain($title) . "</title>\n";
-  $output .= ' <link>' . check_url($link) . "</link>\n";
+  $output .= ' <link>' . check_plain($link) . "</link>\n";
 
   // The RSS 2.0 "spec" doesn't indicate HTML can be used in the description.
   // We strip all HTML tags, but need to prevent double encoding from properly
@@ -1511,7 +1538,7 @@ function format_rss_channel($title, $lin
 function format_rss_item($title, $link, $description, $args = array()) {
   $output = "<item>\n";
   $output .= ' <title>' . check_plain($title) . "</title>\n";
-  $output .= ' <link>' . check_url($link) . "</link>\n";
+  $output .= ' <link>' . check_plain($link) . "</link>\n";
   $output .= ' <description>' . check_plain($description) . "</description>\n";
   $output .= format_xml_elements($args);
   $output .= "</item>\n";
@@ -1961,7 +1988,7 @@ function url($path = NULL, array $option
     // Note: we could use url_is_external($path) here, but that would
     // require another function call, and performance inside url() is critical.
     $colonpos = strpos($path, ':');
-    $options['external'] = ($colonpos !== FALSE && !preg_match('![/?#]!', substr($path, 0, $colonpos)) && filter_xss_bad_protocol($path, FALSE) == check_plain($path));
+    $options['external'] = ($colonpos !== FALSE && !preg_match('![/?#]!', substr($path, 0, $colonpos)) && filter_xss_bad_protocol($path, FALSE) == $path);
   }
 
   // Preserve the original path before altering or aliasing.
@@ -2081,7 +2108,7 @@ function url_is_external($path) {
   $colonpos = strpos($path, ':');
   // Only call the slow filter_xss_bad_protocol if $path contains a ':'
   // before any / ? or #.
-  return $colonpos !== FALSE && !preg_match('![/?#]!', substr($path, 0, $colonpos)) && filter_xss_bad_protocol($path, FALSE) == check_plain($path);
+  return $colonpos !== FALSE && !preg_match('![/?#]!', substr($path, 0, $colonpos)) && filter_xss_bad_protocol($path, FALSE) == $path;
 }
 
 /**
@@ -2136,6 +2163,10 @@ function drupal_attributes(array $attrib
  * internal links output by modules should be generated by this function if
  * possible.
  *
+ * This function is not responsible for stripping bad protocols from URLs
+ * that come from user input. You should use check_url() function in that
+ * case.
+ *
  * @param $text
  *   The link text for the anchor tag.
  * @param $path
Index: includes/form.inc
===================================================================
RCS file: /cvs/drupal/drupal/includes/form.inc,v
retrieving revision 1.452
diff -u -p -r1.452 form.inc
--- includes/form.inc	13 Apr 2010 15:23:02 -0000	1.452
+++ includes/form.inc	23 Apr 2010 16:47:36 -0000
@@ -2834,7 +2834,7 @@ function theme_textfield($variables) {
   if ($element['#autocomplete_path'] && drupal_valid_path($element['#autocomplete_path'])) {
     drupal_add_js('misc/autocomplete.js');
     $class[] = 'form-autocomplete';
-    $extra =  '<input class="autocomplete" type="hidden" id="' . $element['#id'] . '-autocomplete" value="' . check_url(url($element['#autocomplete_path'], array('absolute' => TRUE))) . '" disabled="disabled" />';
+    $extra =  '<input class="autocomplete" type="hidden" id="' . $element['#id'] . '-autocomplete" value="' . check_plain(url($element['#autocomplete_path'], array('absolute' => TRUE))) . '" disabled="disabled" />';
   }
   _form_set_class($element, $class);
 
@@ -2856,7 +2856,7 @@ function theme_textfield($variables) {
 function theme_form($variables) {
   $element = $variables['element'];
   // Anonymous div to satisfy XHTML compliance.
-  $action = $element['#action'] ? 'action="' . check_url($element['#action']) . '" ' : '';
+  $action = $element['#action'] ? 'action="' . check_plain($element['#action']) . '" ' : '';
   return '<form ' . $action . ' accept-charset="UTF-8" method="' . $element['#method'] . '" id="' . $element['#id'] . '"' . drupal_attributes($element['#attributes']) . ">\n<div>" . $element['#children'] . "\n</div></form>\n";
 }
 
Index: includes/install.core.inc
===================================================================
RCS file: /cvs/drupal/drupal/includes/install.core.inc,v
retrieving revision 1.11
diff -u -p -r1.11 install.core.inc
--- includes/install.core.inc	22 Apr 2010 09:13:53 -0000	1.11
+++ includes/install.core.inc	23 Apr 2010 16:47:36 -0000
@@ -723,7 +723,7 @@ function install_verify_requirements(&$i
     if ($install_state['interactive']) {
       drupal_set_title(st('Requirements problem'));
       $status_report = theme('status_report', array('requirements' => $requirements));
-      $status_report .= st('Check the error messages and <a href="!url">proceed with the installation</a>.', array('!url' => check_url(request_uri())));
+      $status_report .= st('Check the error messages and <a href="@url">proceed with the installation</a>.', array('@url' => request_uri()));
       return $status_report;
     }
     else {
Index: includes/theme.inc
===================================================================
RCS file: /cvs/drupal/drupal/includes/theme.inc,v
retrieving revision 1.589
diff -u -p -r1.589 theme.inc
--- includes/theme.inc	13 Apr 2010 15:23:02 -0000	1.589
+++ includes/theme.inc	23 Apr 2010 16:47:37 -0000
@@ -1523,7 +1523,7 @@ function theme_image($variables) {
   if (!$getsize || (is_file($path) && (list($width, $height, $type, $image_attributes) = @getimagesize($path)))) {
     $attributes = drupal_attributes($attributes);
     $url = file_create_url($path);
-    return '<img src="' . check_url($url) . '" alt="' . check_plain($alt) . '" title="' . check_plain($title) . '" ' . (isset($image_attributes) ? $image_attributes : '') . $attributes . ' />';
+    return '<img src="' . check_plain($url) . '" alt="' . check_plain($alt) . '" title="' . check_plain($title) . '" ' . (isset($image_attributes) ? $image_attributes : '') . $attributes . ' />';
   }
 }
 
@@ -1867,7 +1867,7 @@ function theme_item_list($variables) {
  *   - url: The url for the link.
  */
 function theme_more_help_link($variables) {
-  return '<div class="more-help-link">' . t('<a href="@link">More help</a>', array('@link' => check_url($variables['url']))) . '</div>';
+  return '<div class="more-help-link">' . l(t('More help'), $variables['url']) . '</div>';
 }
 
 /**
@@ -1881,7 +1881,7 @@ function theme_more_help_link($variables
 function theme_feed_icon($variables) {
   $text = t('Subscribe to @feed-title', array('@feed-title' => $variables['title']));
   if ($image = theme('image', array('path' => 'misc/feed.png', 'alt' => $text))) {
-    return '<a href="' . check_url($variables['url']) . '" title="' . $text . '" class="feed-icon">' . $image . '</a>';
+    return l($image, $variables['url'], array('html' => TRUE, 'attributes' => array('class' => array('feed-icon'), 'title' => $text)));
   }
 }
 
@@ -1931,7 +1931,7 @@ function theme_html_tag($variables) {
  *   - title: A descriptive verb for the link, like 'Read more'.
  */
 function theme_more_link($variables) {
-  return '<div class="more-link">' . t('<a href="@link" title="@title">More</a>', array('@link' => check_url($variables['url']), '@title' => $variables['title'])) . '</div>';
+  return '<div class="more-link">' . l(t('More'), $variables['url'], array('attributes' => array('title' => $variables['title']))) . '</div>';
 }
 
 /**
@@ -2176,7 +2176,7 @@ function template_preprocess_html(&$vari
   if (theme_get_setting('toggle_favicon')) {
     $favicon = theme_get_setting('favicon');
     $type = theme_get_setting('favicon_mimetype');
-    drupal_add_html_head_link(array('rel' => 'shortcut icon', 'href' => check_url($favicon), 'type' => $type));
+    drupal_add_html_head_link(array('rel' => 'shortcut icon', 'href' => check_url($favicon, FALSE), 'type' => $type));
   }
 
   // Construct page title.
@@ -2360,7 +2360,7 @@ function template_preprocess_maintenance
   if (theme_get_setting('toggle_favicon')) {
     $favicon = theme_get_setting('favicon');
     $type = theme_get_setting('favicon_mimetype');
-    drupal_add_html_head_link(array('rel' => 'shortcut icon', 'href' => check_url($favicon), 'type' => $type));
+    drupal_add_html_head_link(array('rel' => 'shortcut icon', 'href' => check_url($favicon, FALSE), 'type' => $type));
   }
 
   global $theme;
Index: includes/theme.maintenance.inc
===================================================================
RCS file: /cvs/drupal/drupal/includes/theme.maintenance.inc,v
retrieving revision 1.58
diff -u -p -r1.58 theme.maintenance.inc
--- includes/theme.maintenance.inc	20 Apr 2010 08:19:01 -0000	1.58
+++ includes/theme.maintenance.inc	23 Apr 2010 16:47:37 -0000
@@ -157,7 +157,7 @@ function theme_install_page($variables) 
     $title = count($messages['error']) > 1 ? st('The following errors must be resolved before you can continue the installation process') : st('The following error must be resolved before you can continue the installation process');
     $variables['messages'] .= '<h3>' . $title . ':</h3>';
     $variables['messages'] .= theme('status_messages', array('display' => 'error'));
-    $variables['content'] .= '<p>' . st('Check the error messages and <a href="!url">try again</a>.', array('!url' => check_url(request_uri()))) . '</p>';
+    $variables['content'] .= '<p>' . st('Check the error messages and <a href="@url">try again</a>.', array('@url' => request_uri())) . '</p>';
   }
 
   // Special handling of warning messages
Index: modules/comment/comment.tokens.inc
===================================================================
RCS file: /cvs/drupal/drupal/modules/comment/comment.tokens.inc,v
retrieving revision 1.11
diff -u -p -r1.11 comment.tokens.inc
--- modules/comment/comment.tokens.inc	20 Apr 2010 09:48:06 -0000	1.11
+++ modules/comment/comment.tokens.inc	23 Apr 2010 16:47:37 -0000
@@ -172,7 +172,7 @@ function comment_tokens($type, $tokens, 
           break;
 
         case 'homepage':
-          $replacements[$original] = $sanitize ? filter_xss_bad_protocol($comment->homepage) : $comment->homepage;
+          $replacements[$original] = $sanitize ? check_url($comment->homepage) : $comment->homepage;
           break;
 
         case 'title':
Index: modules/search/search.pages.inc
===================================================================
RCS file: /cvs/drupal/drupal/modules/search/search.pages.inc,v
retrieving revision 1.18
diff -u -p -r1.18 search.pages.inc
--- modules/search/search.pages.inc	13 Apr 2010 15:23:03 -0000	1.18
+++ modules/search/search.pages.inc	23 Apr 2010 16:47:37 -0000
@@ -90,7 +90,7 @@ function template_preprocess_search_resu
  */
 function template_preprocess_search_result(&$variables) {
   $result = $variables['result'];
-  $variables['url'] = check_url($result['link']);
+  $variables['url'] = check_plain($result['link']);
   $variables['title'] = check_plain($result['title']);
 
   $info = array();
Index: modules/simpletest/tests/common.test
===================================================================
RCS file: /cvs/drupal/drupal/modules/simpletest/tests/common.test,v
retrieving revision 1.109
diff -u -p -r1.109 common.test
--- modules/simpletest/tests/common.test	11 Apr 2010 18:33:44 -0000	1.109
+++ modules/simpletest/tests/common.test	23 Apr 2010 16:47:38 -0000
@@ -82,7 +82,10 @@ class CommonURLUnitTest extends DrupalWe
     $text = $this->randomName();
     $path = "<SCRIPT>alert('XSS')</SCRIPT>";
     $link = l($text, $path);
-    $sanitized_path = check_url(url($path));
+    // Not check_url(), because l() is not responsible for stripping bad
+    // protocols. Code that calls l() with a path that comes from user input
+    // needs to do that.
+    $sanitized_path = check_plain(url($path));
     $this->assertTrue(strpos($link, $sanitized_path) !== FALSE, t('XSS attack @path was filtered', array('@path' => $path)));
   }
 
@@ -314,7 +317,7 @@ class CommonXssUnitTest extends DrupalUn
   public static function getInfo() {
     return array(
       'name' => 'String filtering tests',
-      'description' => 'Confirm that check_plain() and filter_xss() work correctly, including invalid multi-byte sequences.',
+      'description' => 'Confirm that check_plain(), filter_xss(), and check_url() work correctly, including invalid multi-byte sequences.',
       'group' => 'System',
     );
   }
@@ -341,6 +344,20 @@ class CommonXssUnitTest extends DrupalUn
      $text = check_plain("<script>");
      $this->assertEqual($text, '&lt;script&gt;', 'check_plain() escapes &lt;script&gt;');
   }
+
+  /**
+   * Check that harmful protocols are stripped.
+   */
+  function testBadProtocolStripping() {
+    // Ensure that check_url() strips out harmful protocols, and can be used by
+    // theme functions and templates to output an HTML-safe string as well as by
+    // intermediary functions that require a plain text string to be returned.
+    $url = 'javascript:http://www.google.com/#&q=more';
+    $expected_html = 'http://www.google.com/#&amp;q=more';
+    $expected_plain = 'http://www.google.com/#&q=more';
+    $this->assertIdentical(check_url($url), $expected_html, t('check_url() filters a URL and sanitizes it'));
+    $this->assertIdentical(check_url($url, FALSE), $expected_plain, t('check_url() filters a URL and returns plain text'));
+  }
 }
 
 class CommonSizeTestCase extends DrupalUnitTestCase {