diff --git a/core/lib/Drupal/Component/Utility/FormattableString.php b/core/lib/Drupal/Component/Utility/FormattableString.php index ea7861c..66cbd88 100644 --- a/core/lib/Drupal/Component/Utility/FormattableString.php +++ b/core/lib/Drupal/Component/Utility/FormattableString.php @@ -10,28 +10,33 @@ /** * Formats a string for HTML display by replacing variable placeholders. * - * When cast to a string it replaces variable placeholders in the string with - * the arguments passed in during construction and escapes the values so they - * can be safely displayed as HTML. It should be used on any unknown text that - * is intended to be printed to an HTML page (especially text that may have come - * from untrusted users, since in that case it prevents cross-site scripting and - * other security problems). - * - * This class is not intended for passing arbitrary user input into any HTML - * attribute value, as only URL attributes such as "src" and "href" are - * supported (using ":variable"). Never use this method on unsafe HTML - * attributes such as "on*" and "style" and take care when using this with - * unsupported attributes such as "title" or "alt" as this can lead to - * unexpected output. + * When cast to a string, this object replaces variable placeholders in the + * string with the arguments passed in during construction and escapes the + * values so they can be safely displayed as HTML. See the documentation of + * \Drupal\Component\Utility\PlaceholderTrait::placeholderFormat() for + * details on the supported placeholders and how to use them securely. Incorrect + * use of this class can result in security vulnerabilities. * * In most cases, you should use TranslatableString or PluralTranslatableString * rather than this object, since they will translate the text (on - * non-English-only sites) in addition to formatting it. + * non-English-only sites) in addition to formatting it. Variables concatenated + * without the insertion of language-specific words or punctuation are some + * examples where translation is not applicable and using this class directly + * directly is appropriate. + * + * This class is designed for formatting messages that are mostly text, not as + * an HTML template language. As such, the passed-in string: + * - Should contain minimal HTML. + * - Should not be used within the "<" and ">" of an HTML tag, such as in HTML + * attribute values. This would be a security risk. + * To build HTML that cannot meet these restrictions, use an HTML template + * language such as Twig, rather than this class. * * @ingroup sanitization * * @see \Drupal\Core\StringTranslation\TranslatableString * @see \Drupal\Core\StringTranslation\PluralTranslatableString + * @see \Drupal\Component\Utility\PlaceholderTrait::placeholderFormat() */ class FormattableString implements SafeStringInterface { diff --git a/core/lib/Drupal/Component/Utility/PlaceholderTrait.php b/core/lib/Drupal/Component/Utility/PlaceholderTrait.php index e2db26b..a57dbf8 100644 --- a/core/lib/Drupal/Component/Utility/PlaceholderTrait.php +++ b/core/lib/Drupal/Component/Utility/PlaceholderTrait.php @@ -15,35 +15,86 @@ /** * Formats a string by replacing variable placeholders. * - * This trait is not intended for passing arbitrary user input into any HTML - * attribute value, as only URL attributes such as "src" and "href" are - * supported (using ":variable"). Never use this method on unsafe HTML - * attributes such as "on*" and "style" and take care when using this with - * unsupported attributes such as "title" or "alt" as this can lead to - * unexpected and unsafe output. + * This trait is designed for formatting messages that are mostly text, not as + * an HTML template language. As such, the passed-in string: + * - Should contain minimal HTML. + * - Should not be used within the "<" and ">" of an HTML tag, such as in HTML + * attribute values. This would be a security risk. Examples: + * @code + * // Insecure (placeholder within "<" and ">"): + * $this->placeholderFormat('<@variable>text', ['@variable' => $variable]); + * // Insecure (placeholder within "<" and ">"): + * $this->placeholderFormat('link text', ['@variable' => $variable]); + * // Insecure (placeholder within "<" and ">"): + * $this->placeholderFormat('link text', ['@variable' => $variable]); + * @endcode + * Only the "href" attribute is supported via the special ":variable" + * placeholder, to allow simple links to be inserted: + * @code + * // Secure (usage of ":variable" placeholder for href attribute): + * $this->placeholderFormat('link text', [':variable' , $variable]); + * // Secure (usage of ":variable" placeholder for href attribute): + * $this->placeholderFormat('link text', [':variable' => $variable]); + * // Insecure (the "@variable" placeholder does not filter dangerous + * // attributes): + * $this->placeholderFormat('link text', [':variable' => $variable]); + * // Insecure ("@variable" placeholder within "<" and ">"): + * $this->placeholderFormat('link text', [':url' => $url, '@variable' => $variable]); + * @endcode + * To build non-minimal HTML, use an HTML template language such as Twig, + * rather than this trait. * * @param string $string - * A string containing placeholders. + * A string containing placeholders. The string itself is expected to be + * safe and correct HTML. Any unsafe content must be in $args and + * inserted via placeholders. * @param array $args - * An associative array of replacements to make. Occurrences in $string of - * any key in $args are replaced with the corresponding value, after - * optional 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. - * - %variable: Escaped to HTML just like @variable, but also wrapped in - * tags, which makes the following HTML code: + * An associative array of replacements. Each array key should be the same + * as a placeholder in $string. The corresponding value should be a string + * or an object that implements + * \Drupal\Component\Utility\SafeStringInterface. The value replaces the + * placeholder in $string. Sanitization and formatting will be done before + * replacement. The type of sanitization and formatting depends on the first + * character of the key: + * - @variable: Use when the return value is to be used as HTML. When the + * placeholder replacement value is: + * - A string, it will be sanitized using + * \Drupal\Component\Utility\Html::escape(). + * - A SafeStringInterface object, the object will be cast to a string and + * not sanitized. + * - A SafeStringInterface object cast to a string, forces sanitization. + * Since it is not known what method a SafeStringInterface object could + * have been santitized with, to force sanitization to happen, the + * caller method could cast the object as a string before passing it in + * as the replacement value. + * Use this placeholder as the default choice for anything displayed on + * the site, but not within HTML attributes, JavaScript, or CSS. Doing so + * is a security risk. + * - %variable: Use when the return value is to be used inside HTML + * fragments similarly to @variable, but also wrapped in tags. A call + * like: + * @code + * $string = "%output_text"; + * $arguments = ['output_text' => 'text output here.']; + * $this->placeholderFormat($string, $arguments); + * @endcode + * makes the following HTML code: * @code * text output here. * @endcode - * As with @variable, do not use this within HTML attributes. - * - :variable: Escaped to HTML using Html::escape() and filtered for - * dangerous protocols using UrlHelper::stripDangerousProtocols(). Use - * this when passing in a URL, such as when using the "src" or "href" - * attributes, ensuring the value is always wrapped in quotes: - * - Secure: @variable - * - Insecure: @variable + * As with @variable, do not use this within HTML attributes, JavaScript, + * or CSS. Doing so is a security risk. + * - :variable: Return value is escaped with + * \Drupal\Component\Utility\Html::escape() and filtered for dangerous + * protocols using UrlHelper::stripDangerousProtocols(). Use this when + * using the "href" attribute, ensuring the value is always wrapped in + * quotes: + * @code + * // Secure (with quotes): + * $this->placeholderFormat('@variable', [':url' => $url, @variable => $variable]); + * // Insecure (without quotes): + * $this->placeholderFormat('@variable', [':url' => $url, @variable => $variable]); + * @endcode * When ":variable" comes from arbitrary user input, the result is secure, * but not guaranteed to be a valid URL (which means the resulting output * could fail HTML validation). To guarantee a valid URL, use @@ -52,7 +103,7 @@ * ":variable" placeholder. * * @return string - * The string with the placeholders replaced. + * A formatted HTML string with the placeholders replaced. * * @ingroup sanitization * @@ -68,21 +119,40 @@ protected static function placeholderFormat($string, array $args) { foreach ($args as $key => $value) { switch ($key[0]) { case '@': - // Escaped only. + // Escape if the value is not an object from a class that implements + // \Drupal\Component\Utility\SafeStringInterface, for example strings + // will be escaped. + // \Drupal\Component\Utility\SafeMarkup\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(). $args[$key] = static::placeholderEscape($value); break; case ':': - // URL attributes must be escaped unconditionally (even if they were - // already marked safe) since content that has been filtered for XSS - // can still contain characters that are unsafe for use in attributes. - // @todo decide what to do about non-URL attribute values (#2570431) - $args[$key] = Html::escape(UrlHelper::stripDangerousProtocols($value)); + // Strip URL protocols that can be XSS vectors. + $value = UrlHelper::stripDangerousProtocols($value); + // Escape unconditionally, without checking + // \Drupal\Component\Utility\SafeMarkup\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 + // \Drupal\Component\Utility\OutputStrategyInterface::renderFromHtml() + // on it prior to passing it in as a placeholder value of this type. + // @todo Add some advice and stronger warnings. + // https://www.drupal.org/node/2569041. + $args[$key] = Html::escape($value); break; case '%': default: - // Escaped and placeholder. + // Similarly to @, escape non-safe values. Also, add wrapping markup + // in order to render as a placeholder. Not for use within attributes, + // per the warning above about + // \Drupal\Component\Utility\SafeMarkup\SafeMarkup::isSafe() and also + // due to the wrapping markup. $args[$key] = '' . static::placeholderEscape($value) . ''; break; } diff --git a/core/lib/Drupal/Core/StringTranslation/TranslatableString.php b/core/lib/Drupal/Core/StringTranslation/TranslatableString.php index 0dbb3a6..6d587a1 100644 --- a/core/lib/Drupal/Core/StringTranslation/TranslatableString.php +++ b/core/lib/Drupal/Core/StringTranslation/TranslatableString.php @@ -19,6 +19,7 @@ * This is useful for using translation in very low level subsystems like entity * definition and stream wrappers. * + * @see \Drupal\Component\Utility\PlaceholderTrait::placeholderFormat() * @see \Drupal\Core\StringTranslation\TranslationManager::translate() * @see \Drupal\Core\StringTranslation\TranslationManager::translateString() * @see \Drupal\Core\Annotation\Translation