diff --git a/core/includes/common.inc b/core/includes/common.inc index 3545e05..3caedbb 100644 --- a/core/includes/common.inc +++ b/core/includes/common.inc @@ -2869,6 +2869,19 @@ function drupal_render(&$elements, $is_root_call = FALSE) { // Assume that if #theme is set it represents an implemented hook. $theme_is_implemented = isset($elements['#theme']); + // Check the elements for insecure HTML and pass through sanitization. + if (isset($elements)) { + $markup_keys = array( + '#description', + '#field_prefix', + '#field_suffix', + ); + foreach ($markup_keys as $key) { + if (!empty($elements[$key]) && is_scalar($elements[$key])) { + $elements[$key] = SafeMarkup::checkAdminXss($elements[$key]); + } + } + } // Call the element's #theme function if it is set. Then any children of the // element have to be rendered there. If the internal #render_children @@ -2950,8 +2963,9 @@ function drupal_render(&$elements, $is_root_call = FALSE) { // with how render cached output gets stored. This ensures that // #post_render_cache callbacks get the same data to work with, no matter if // #cache is disabled, #cache is enabled, there is a cache hit or miss. - $prefix = isset($elements['#prefix']) ? $elements['#prefix'] : ''; - $suffix = isset($elements['#suffix']) ? $elements['#suffix'] : ''; + $prefix = isset($elements['#prefix']) ? SafeMarkup::checkAdminXss($elements['#prefix']) : ''; + $suffix = isset($elements['#suffix']) ? SafeMarkup::checkAdminXss($elements['#suffix']) : ''; + $elements['#markup'] = $prefix . $elements['#children'] . $suffix; // We've rendered this element (and its subtree!), now update the stack. diff --git a/core/lib/Drupal/Component/Utility/SafeMarkup.php b/core/lib/Drupal/Component/Utility/SafeMarkup.php index dc0a6a1..3d46f8f 100644 --- a/core/lib/Drupal/Component/Utility/SafeMarkup.php +++ b/core/lib/Drupal/Component/Utility/SafeMarkup.php @@ -137,6 +137,22 @@ public static function escape($string) { } /** + * Applies a very permissive XSS/HTML filter for admin-only use. + * + * @param $string + * A string. + * + * @return string + * The escaped string. If $string was already set as safe with + * SafeString::set, it won't be escaped again. + * + * @see \Drupal\Component\Utility\Xss\filterAdmin + */ + public static function checkAdminXss($string) { + return static::isSafe($string) ? $string : Xss::filterAdmin($string); + } + + /** * Retrieves all strings currently marked as safe. * * This is useful for the batch and form APIs, where it is important to diff --git a/core/lib/Drupal/Core/Form/FormBuilder.php b/core/lib/Drupal/Core/Form/FormBuilder.php index dd2bcd8..da6d8ad 100644 --- a/core/lib/Drupal/Core/Form/FormBuilder.php +++ b/core/lib/Drupal/Core/Form/FormBuilder.php @@ -10,6 +10,7 @@ use Drupal\Component\Utility\Crypt; use Drupal\Component\Utility\Html; use Drupal\Component\Utility\NestedArray; +use Drupal\Component\Utility\SafeMarkup; use Drupal\Component\Utility\String; use Drupal\Component\Utility\UrlHelper; use Drupal\Core\Access\CsrfTokenGenerator; @@ -799,7 +800,6 @@ public function doBuildForm($form_id, &$element, FormStateInterface &$form_state $element[$key] = $this->doBuildForm($form_id, $element[$key], $form_state); $count++; } - // The #after_build flag allows any piece of a form to be altered // after normal input parsing has been completed. if (isset($element['#after_build']) && !isset($element['#after_build_done'])) { diff --git a/core/lib/Drupal/Core/Render/Element/HtmlTag.php b/core/lib/Drupal/Core/Render/Element/HtmlTag.php index 851bbf9..69eb540 100644 --- a/core/lib/Drupal/Core/Render/Element/HtmlTag.php +++ b/core/lib/Drupal/Core/Render/Element/HtmlTag.php @@ -139,7 +139,8 @@ public static function preRenderConditionalComments($element) { $expression = '!IE'; } else { - $expression = $browsers['IE']; + // The IE expression might contain some user input data. + $expression = SafeMarkup::checkAdminXss($browsers['IE']); } // Wrap the element's potentially existing #prefix and #suffix properties with @@ -147,19 +148,23 @@ public static function preRenderConditionalComments($element) { // by Internet Explorer only. To control the rendering by other browsers, // either the "downlevel-hidden" or "downlevel-revealed" technique must be // used. See http://en.wikipedia.org/wiki/Conditional_comment for details. - $element += array( - '#prefix' => '', - '#suffix' => '', - ); + + // Ensure what we are dealing with is safe. + // This would be done later anyway in drupal_render(). + $prefix = isset($elements['#prefix']) ? SafeMarkup::checkAdminXss($elements['#prefix']) : ''; + $suffix = isset($elements['#suffix']) ? SafeMarkup::checkAdminXss($elements['#suffix']) : ''; + + // Now calling SafeMarkup::set is safe, because we ensured the + // data coming in was at least admin escaped. if (!$browsers['!IE']) { // "downlevel-hidden". - $element['#prefix'] = "\n\n"; + $element['#prefix'] = SafeMarkup::set("\n\n"); } else { // "downlevel-revealed". - $element['#prefix'] = "\n\n" . $element['#prefix']; - $element['#suffix'] .= "\n"; + $element['#prefix'] = SafeMarkup::set("\n\n" . $prefix); + $element['#suffix'] = SafeMarkup::set($suffix . "\n"); } return $element; diff --git a/core/lib/Drupal/Core/Render/Element/MachineName.php b/core/lib/Drupal/Core/Render/Element/MachineName.php index 24ac3a4..f0c3768 100644 --- a/core/lib/Drupal/Core/Render/Element/MachineName.php +++ b/core/lib/Drupal/Core/Render/Element/MachineName.php @@ -8,7 +8,6 @@ namespace Drupal\Core\Render\Element; use Drupal\Component\Utility\NestedArray; -use Drupal\Component\Utility\SafeMarkup; use Drupal\Core\Form\FormStateInterface; use Drupal\Core\Language\LanguageInterface; @@ -152,13 +151,13 @@ public static function processMachineName(&$element, FormStateInterface $form_st $element['#machine_name']['suffix'] = '#' . $suffix_id; if ($element['#machine_name']['standalone']) { - $element['#suffix'] = SafeMarkup::set($element['#suffix'] . '  '); + $element['#suffix'] = $element['#suffix'] . '  '; } else { // Append a field suffix to the source form element, which will contain // the live preview of the machine name. $source += array('#field_suffix' => ''); - $source['#field_suffix'] = SafeMarkup::set($source['#field_suffix'] . '  '); + $source['#field_suffix'] = $source['#field_suffix'] . '  '; $parents = array_merge($element['#machine_name']['source'], array('#field_suffix')); NestedArray::setValue($form_state->getCompleteForm(), $parents, $source['#field_suffix']); diff --git a/core/modules/editor/editor.admin.inc b/core/modules/editor/editor.admin.inc index 85006eb..83bc2cf 100644 --- a/core/modules/editor/editor.admin.inc +++ b/core/modules/editor/editor.admin.inc @@ -6,7 +6,6 @@ */ use Drupal\Core\StreamWrapper\StreamWrapperInterface; -use Drupal\Component\Utility\SafeMarkup; use Drupal\editor\Entity\Editor; /** @@ -93,8 +92,8 @@ function editor_image_upload_settings_form(Editor $editor) { $form['max_dimensions'] = array( '#type' => 'item', '#title' => t('Maximum dimensions'), - '#field_prefix' => SafeMarkup::set('
'), - '#field_suffix' => SafeMarkup::set('
'), + '#field_prefix' => '
', + '#field_suffix' => '
', '#description' => t('Images larger than these dimensions will be scaled down.'), '#states' => $show_if_image_uploads_enabled, ); diff --git a/core/modules/field_ui/src/Tests/FieldUiTestBase.php b/core/modules/field_ui/src/Tests/FieldUiTestBase.php index 962acec..314cf5b 100644 --- a/core/modules/field_ui/src/Tests/FieldUiTestBase.php +++ b/core/modules/field_ui/src/Tests/FieldUiTestBase.php @@ -105,6 +105,7 @@ function fieldUIAddExistingField($bundle_path, $initial_edit, $field_edit = arra // First step : 'Re-use existing field' on the 'Manage fields' page. $this->drupalPostForm("$bundle_path/fields", $initial_edit, t('Save')); + $this->assertNoRaw('<', 'The page does not have double escaped HTML tags.'); // Second step : 'Field settings' form. $this->drupalPostForm(NULL, $field_edit, t('Save settings')); diff --git a/core/modules/options/src/Tests/OptionsFieldUITest.php b/core/modules/options/src/Tests/OptionsFieldUITest.php index e3b4684..40343bf 100644 --- a/core/modules/options/src/Tests/OptionsFieldUITest.php +++ b/core/modules/options/src/Tests/OptionsFieldUITest.php @@ -278,6 +278,7 @@ protected function createOptionsField($type) { function assertAllowedValuesInput($input_string, $result, $message) { $edit = array('field_storage[settings][allowed_values]' => $input_string); $this->drupalPostForm($this->admin_path, $edit, t('Save field settings')); + $this->assertNoRaw('<', 'The page does not have double escaped HTML tags.'); if (is_string($result)) { $this->assertText($result, $message); diff --git a/core/modules/rdf/rdf.module b/core/modules/rdf/rdf.module index 371be2c..c3fce4f 100644 --- a/core/modules/rdf/rdf.module +++ b/core/modules/rdf/rdf.module @@ -498,14 +498,14 @@ function rdf_preprocess_comment(&$variables) { } // Adds RDF metadata markup above comment body. if (!empty($variables['rdf_metadata_attributes'])) { - if (!isset($variables['content']['comment_body']['#prefix'])) { - $variables['content']['comment_body']['#prefix'] = ''; - } $rdf_metadata = array( '#theme' => 'rdf_metadata', '#metadata' => $variables['rdf_metadata_attributes'], ); - $variables['content']['comment_body']['#prefix'] = drupal_render($rdf_metadata) . $variables['content']['comment_body']['#prefix']; + if (!empty($variables['content']['comment_body']['#prefix'])) { + $rdf_metadata['#suffix'] = $variables['content']['comment_body']['#prefix']; + } + $variables['content']['comment_body']['#prefix'] = drupal_render($rdf_metadata); } } diff --git a/core/modules/system/src/Tests/Common/RenderTest.php b/core/modules/system/src/Tests/Common/RenderTest.php index 381ff8d..739e3b5 100644 --- a/core/modules/system/src/Tests/Common/RenderTest.php +++ b/core/modules/system/src/Tests/Common/RenderTest.php @@ -808,10 +808,10 @@ function testDrupalRenderRenderCachePlaceholder() { ), ), '#markup' => $placeholder, - '#prefix' => '', - '#suffix' => '' + '#prefix' => '
',
+      '#suffix' => '
', ); - $expected_output = '' . $context['bar'] . ''; + $expected_output = '
' . $context['bar'] . '
'; // #cache disabled. $element = $test_element; @@ -852,7 +852,7 @@ function testDrupalRenderRenderCachePlaceholder() { $this->assertIdentical($token, $expected_token, 'The tokens are identical'); // Verify the token is in the cached element. $expected_element = array( - '#markup' => '', + '#markup' => '
', '#attached' => array(), '#post_render_cache' => array( 'common_test_post_render_cache_placeholder' => array( @@ -895,11 +895,11 @@ function testDrupalRenderChildElementRenderCachePlaceholder() { ], ], '#markup' => $placeholder, - '#prefix' => '', - '#suffix' => '' + '#prefix' => '
',
+        '#suffix' => '
' ], ]; - $expected_output = '' . $context['bar'] . '' . "\n"; + $expected_output = '
' . $context['bar'] . '
' . "\n"; // #cache disabled. $element = $test_element; @@ -943,7 +943,7 @@ function testDrupalRenderChildElementRenderCachePlaceholder() { $this->assertIdentical($token, $expected_token, 'The tokens are identical for the child element'); // Verify the token is in the cached element. $expected_element = array( - '#markup' => '', + '#markup' => '
', '#attached' => array(), '#post_render_cache' => array( 'common_test_post_render_cache_placeholder' => array( @@ -969,7 +969,7 @@ function testDrupalRenderChildElementRenderCachePlaceholder() { $this->assertIdentical($token, $expected_token, 'The tokens are identical for the parent element'); // Verify the token is in the cached element. $expected_element = array( - '#markup' => '' . "\n", + '#markup' => '
' . "\n", '#attached' => array(), '#post_render_cache' => array( 'common_test_post_render_cache_placeholder' => array( @@ -999,7 +999,7 @@ function testDrupalRenderChildElementRenderCachePlaceholder() { $this->assertIdentical($token, $expected_token, 'The tokens are identical for the child element'); // Verify the token is in the cached element. $expected_element = array( - '#markup' => '', + '#markup' => '
', '#attached' => array(), '#post_render_cache' => array( 'common_test_post_render_cache_placeholder' => array( diff --git a/core/modules/system/system.admin.inc b/core/modules/system/system.admin.inc index 0231667..0430cd9 100644 --- a/core/modules/system/system.admin.inc +++ b/core/modules/system/system.admin.inc @@ -259,7 +259,7 @@ function theme_system_modules_details($variables) { '#type' => 'details', '#title' => SafeMarkup::set(' ' . drupal_render($module['description']) . ''), '#attributes' => array('id' => $module['enable']['#id'] . '-description'), - '#description' => SafeMarkup::set($description), + '#description' => $description, ); $col4 = drupal_render($details); $row[] = array('class' => array('description', 'expand'), 'data' => $col4);