diff --git a/core/lib/Drupal/Component/Utility/PlaceholderTrait.php b/core/lib/Drupal/Component/Utility/PlaceholderTrait.php index 2655b25..cc8d28f 100644 --- a/core/lib/Drupal/Component/Utility/PlaceholderTrait.php +++ b/core/lib/Drupal/Component/Utility/PlaceholderTrait.php @@ -36,8 +36,8 @@ protected static function placeholderFormat($string, array $args, &$safe = TRUE) // Escape if not already safe. SafeMarkup::isSafe() may return TRUE // for content that is safe within HTML fragments, but not within // other contexts, so this placeholder type must not be used within - // HTML attributes, JavaScript or CSS. - // @see \Drupal\Component\Utility\SafeMarkup::format() + // HTML attributes, JavaScript or CSS. See + // \Drupal\Component\Utility\SafeMarkup::format(). if (!SafeMarkup::isSafe($value)) { $args[$key] = Html::escape($value); } @@ -45,7 +45,7 @@ protected static function placeholderFormat($string, array $args, &$safe = TRUE) case '%': default: - // Escape if not already safe and add wrapping markup to render as a + // Escape if not already safe, and add wrapping markup to render as a // placeholder. Not for use within attributes, per the warning above // about SafeMarkup::isSafe() and also due to the wrapping markup. if (!SafeMarkup::isSafe($value)) { @@ -57,13 +57,12 @@ protected static function placeholderFormat($string, array $args, &$safe = TRUE) case ':': // Strip URL protocols that can be XSS vectors. $value = UrlHelper::stripDangerousProtocols($value); - - // This placeholder type is for use in "href" attributes, so it uses - // Html::escape() unconditionally rather than checking - // SafeMarkup::isSafe(). If a caller wants to pass a value that - // is extracted from HTML and therefore is already HTML encoded, it - // must invoke Html::decodeEntities() on it prior to passing it in as - // a placeholder value of this type. + // Escape unconditionally, without checking SafeMarkup::isSafe(). + // This forces characters that are unsafe for use in an "href" HTML + // attribute to be encoded. If a caller wants to pass a value that is + // extracted from HTML and therefore is already HTML encoded, it must + // invoke Html::decodeEntities() on it prior to passing it in as a + // placeholder value of this type. $args[$key] = Html::escape($value); break; diff --git a/core/lib/Drupal/Component/Utility/SafeMarkup.php b/core/lib/Drupal/Component/Utility/SafeMarkup.php index efef1bd..9b93ebb 100644 --- a/core/lib/Drupal/Component/Utility/SafeMarkup.php +++ b/core/lib/Drupal/Component/Utility/SafeMarkup.php @@ -176,17 +176,16 @@ public static function checkPlain($text) { * is not applicable and calling this function directly is appropriate. * * This function is designed for formatting messages that are mostly text, - * not as an HTML template language. As such, it is recommended for $string - * to contain minimal HTML and to not contain any attributes (other than - * "href") with placeholder-replaced values. For any such non-trivial HTML - * building, use an HTML template language, such as Twig, rather than this - * function. - * - * Using this function to put user-generated content into any HTML attribute - * other than "href" is not supported and may not be secure. Only the "href" - * attribute is supported via the ":variable" placeholder type, because - * hyperlinking is a common use case for which requiring an HTML template - * would be inconvenient. + * not as an HTML template language. As such: + * - The passed-in string should contain minimal HTML. + * - Variable placeholders should not be used within HTML attributes; they + * are not safe for this purpose, and doing so represents a security risk. + * Only the "href" attribute is supported via the special ":variable" + * placeholder, to allow simple links to be inserted: + * - Secure: link text + * - Insecure: link text + * For non-trivial HTML building that cannot meet the above restrictions, use + * an HTML template language, such as Twig, rather than this function. * * @param string $string * A string containing placeholders. The string itself is treated as @@ -197,17 +196,30 @@ public static function checkPlain($text) { * any key in $args are replaced with the corresponding value, after * sanitization and formatting. The type of sanitization and * formatting depends on the first character of the key: - * - @variable: Escaped to HTML using Html::escape() unless the value is - * already HTML-safe. Use this as the default choice for anything - * displayed on a page on the site, but not within HTML attributes, not - * within JavaScript and not within CSS as doing so may be insecure. - * - %variable: Escaped to HTML just like @variable, but also wrapped in + * - @variable: Displayed as HTML, with HTML sanitization performed by the + * caller before the string is passed in. If an unsanitized string (that + * has never been marked as HTML-safe) is passed in, it will be + * automatically sanitized using Html::escape(). Use this placeholder as + * the default choice for anything displayed on the site, subject to the + * following restrictions: + * - Never use this within HTML attributes, JavaScript, or CSS. Doing so + * is a security risk. + * - If you need to guarantee that a variable will be output as plain + * text, do not rely on the fallback sanitization; instead, perform the + * sanitization yourself before passing it in. The recommended method + * to sanitize a variable to plain text is to use a render array: + * @code + * $build = ['#plain_text' => $text_to_be_sanitized]; + * $variable = \Drupal::service('renderer')->renderPlain($build); + * $output = SafeMarkup::format('This is plain text: @variable', ['@variable' => $variable]); + * @endcode + * - %variable: Displayed as HTML just like @variable, but also wrapped in * tags, which makes the following HTML code: * @code * text output here. * @endcode * As with @variable, do not use this within HTML attributes, JavaScript - * or CSS as doing so may be insecure. + * or CSS. Doing so is a security risk. * - :variable: Escaped to HTML using Html::escape() and filtered for * dangerous protocols using UrlHelper::stripDangerousProtocols(). Use * this when using the "href" attribute, ensuring the value is always