Sync payment gateway titles fix (#33418)

* Introduce a 'safe_text' field that allows a reduced subset of HTML tags.

* Escape on input as well as output; generalize sanitization function.

* Set reasonable default rules.

* Guard against invalid callbacks (escaping/sanitizing) functions.

* Update plugins/woocommerce/src/Internal/Utilities/HtmlSanitizer.php

Co-authored-by: Peter Fabian <peter.fabian.github@gmail.com>

* Allow alt (accessibility) for img tags; allow class attributes for img and span tags.

* Allow class attr for p tags.

* Use safe_text for payment gateway titles.

* Make HtmlSanitizer available through Utils service provider.

* Update settings code to pull HtmlSanitizer as a service and to use the new sanitize() method.

* Remove `style` from list of allowed attributes.

Allowing arbitrary CSS rules through style could undo the intent of this change, since that would allow a range of positioning and sizing changes to be effected.

* Remove unusued import.

* If no (KSES) rules are specifed, then strip all tags (this is a safer default strategy).

* For better safety, only apply pre-processor callbacks; remove responsibility for trimming of strings.

Applying callbacks to a string after it ahs passed through wp_kses() could (potentially) undo the work done by that function, and result in unexpected tags in the sanitizer's output.

Co-authored-by: barryhughes <3594411+barryhughes@users.noreply.github.com>
Co-authored-by: Peter Fabian <peter.fabian.github@gmail.com>

plugins/woocommerce/includes/abstracts/abstract-wc-payment-gateway.php

@@ -10,6 +10,7 @@
  */
 
 use Automattic\Jetpack\Constants;
+use Automattic\WooCommerce\Internal\Utilities\HtmlSanitizer;
 
 if ( ! defined( 'ABSPATH' ) ) {
 	exit;
@@ -304,7 +305,8 @@ abstract class WC_Payment_Gateway extends WC_Settings_API {
 	 * @return string
 	 */
 	public function get_title() {
-		return apply_filters( 'woocommerce_gateway_title', $this->title, $this->id );
+		$title = wc_get_container()->get( HtmlSanitizer::class )->sanitize( $this->title, HtmlSanitizer::LOW_HTML_BALANCED_TAGS_NO_LINKS );
+		return apply_filters( 'woocommerce_gateway_title', $title, $this->id );
 	}
 
 	/**

plugins/woocommerce/includes/abstracts/abstract-wc-settings-api.php

@@ -9,6 +9,8 @@
 
 defined( 'ABSPATH' ) || exit;
 
+use Automattic\WooCommerce\Internal\Utilities\HtmlSanitizer;
+
 /**
  * WC_Settings_API class.
  */
@@ -852,6 +854,24 @@ abstract class WC_Settings_API {
 		return wp_kses_post( trim( stripslashes( $value ) ) );
 	}
 
+	/**
+	 * Sanitize 'Safe Text' fields.
+	 *
+	 * These fields are similar to regular text fields, but a much  smaller set of HTML tags are allowed. By default,
+	 * this means `<br>`, `<img>`, `<p>` and `<span>` tags.
+	 *
+	 * Note: this is a sanitization method, rather than a validation method (the name is due to some historic naming
+	 * choices).
+	 *
+	 * @param  string $key   Field key (currently unused).
+	 * @param  string $value Posted Value.
+	 *
+	 * @return string
+	 */
+	public function validate_safe_text_field( string $key, string $value ): string {
+		return wc_get_container()->get( HtmlSanitizer::class )->sanitize( $value, HtmlSanitizer::LOW_HTML_BALANCED_TAGS_NO_LINKS );
+	}
+
 	/**
 	 * Validate Price Field.
 	 *

plugins/woocommerce/includes/gateways/bacs/class-wc-gateway-bacs.php

@@ -86,7 +86,7 @@ class WC_Gateway_BACS extends WC_Payment_Gateway {
 			),
 			'title'           => array(
 				'title'       => __( 'Title', 'woocommerce' ),
-				'type'        => 'text',
+				'type'        => 'safe_text',
 				'description' => __( 'This controls the title which the user sees during checkout.', 'woocommerce' ),
 				'default'     => __( 'Direct bank transfer', 'woocommerce' ),
 				'desc_tip'    => true,

plugins/woocommerce/includes/gateways/cheque/class-wc-gateway-cheque.php

@@ -62,7 +62,7 @@ class WC_Gateway_Cheque extends WC_Payment_Gateway {
 			),
 			'title'        => array(
 				'title'       => __( 'Title', 'woocommerce' ),
-				'type'        => 'text',
+				'type'        => 'safe_text',
 				'description' => __( 'This controls the title which the user sees during checkout.', 'woocommerce' ),
 				'default'     => _x( 'Check payments', 'Check payment method', 'woocommerce' ),
 				'desc_tip'    => true,

plugins/woocommerce/includes/gateways/cod/class-wc-gateway-cod.php

@@ -75,7 +75,7 @@ class WC_Gateway_COD extends WC_Payment_Gateway {
 			),
 			'title'              => array(
 				'title'       => __( 'Title', 'woocommerce' ),
-				'type'        => 'text',
+				'type'        => 'safe_text',
 				'description' => __( 'Payment method description that the customer will see on your checkout.', 'woocommerce' ),
 				'default'     => __( 'Cash on delivery', 'woocommerce' ),
 				'desc_tip'    => true,

plugins/woocommerce/includes/gateways/paypal/includes/settings-paypal.php

@@ -16,7 +16,7 @@ return array(
 	),
 	'title'                 => array(
 		'title'       => __( 'Title', 'woocommerce' ),
-		'type'        => 'text',
+		'type'        => 'safe_text',
 		'description' => __( 'This controls the title which the user sees during checkout.', 'woocommerce' ),
 		'default'     => __( 'PayPal', 'woocommerce' ),
 		'desc_tip'    => true,

plugins/woocommerce/src/Internal/DependencyManagement/ServiceProviders/UtilsClassesServiceProvider.php

@@ -7,6 +7,7 @@ namespace Automattic\WooCommerce\Internal\DependencyManagement\ServiceProviders;
 
 use Automattic\WooCommerce\Internal\DependencyManagement\AbstractServiceProvider;
 use Automattic\WooCommerce\Internal\Utilities\DatabaseUtil;
+use Automattic\WooCommerce\Internal\Utilities\HtmlSanitizer;
 
 /**
  * Service provider for the non-static utils classes in the Automattic\WooCommerce\src namespace.
@@ -20,6 +21,7 @@ class UtilsClassesServiceProvider extends AbstractServiceProvider {
 	 */
 	protected $provides = array(
 		DatabaseUtil::class,
+		HtmlSanitizer::class,
 	);
 
 	/**
@@ -27,5 +29,6 @@ class UtilsClassesServiceProvider extends AbstractServiceProvider {
 	 */
 	public function register() {
 		$this->share( DatabaseUtil::class );
+		$this->share( HtmlSanitizer::class );
 	}
 }

plugins/woocommerce/src/Internal/Utilities/HtmlSanitizer.php

@@ -0,0 +1,88 @@
+<?php
+
+namespace Automattic\WooCommerce\Internal\Utilities;
+
+/**
+ * Utility for re-using WP Kses-based sanitization rules.
+ */
+class HtmlSanitizer {
+	/**
+	 * Rules for allowing minimal HTML (breaks, images, paragraphs and spans) without any links.
+	 */
+	public const LOW_HTML_BALANCED_TAGS_NO_LINKS = array(
+		'pre_processors'  => array(
+			'stripslashes',
+			'force_balance_tags',
+		),
+		'wp_kses_rules'   => array(
+			'br'   => true,
+			'img'  => array(
+				'alt'   => true,
+				'class' => true,
+				'src'   => true,
+				'title' => true,
+			),
+			'p'    => array(
+				'class' => true,
+			),
+			'span' => array(
+				'class' => true,
+				'title' => true,
+			),
+		),
+	);
+
+	/**
+	 * Sanitizes the HTML according to the provided rules.
+	 *
+	 * @see wp_kses()
+	 *
+	 * @param string $html HTML string to be sanitized.
+	 * @param array  $sanitizer_rules {
+	 *     Optional and defaults to self::TRIMMED_BALANCED_LOW_HTML_NO_LINKS. Otherwise, one or more of the following
+	 *     keys should be set.
+	 *
+	 *     @type array $pre_processors  Callbacks to run before invoking `wp_kses()`.
+	 *     @type array $wp_kses_rules   Element names and attributes to allow, per `wp_kses()`.
+	 * }
+	 *
+	 * @return string
+	 */
+	public function sanitize( string $html, array $sanitizer_rules = self::LOW_HTML_BALANCED_TAGS_NO_LINKS ): string {
+		if ( isset( $sanitizer_rules['pre_processors'] ) && is_array( $sanitizer_rules['pre_processors'] ) ) {
+			$html = $this->apply_string_callbacks( $sanitizer_rules['pre_processors'], $html );
+		}
+
+		// If no KSES rules are specified, assume all HTML should be stripped.
+		$kses_rules = isset( $sanitizer_rules['wp_kses_rules'] ) && is_array( $sanitizer_rules['wp_kses_rules'] )
+			? $sanitizer_rules['wp_kses_rules']
+			: array();
+
+		return wp_kses( $html, $kses_rules );
+	}
+
+	/**
+	 * Applies callbacks used to process the string before and after wp_kses().
+	 *
+	 * If a callback is invalid we will short-circuit and return an empty string, on the grounds that it is better to
+	 * output nothing than risky HTML. We also call the problem out via _doing_it_wrong() to highlight the problem (and
+	 * increase the chances of this being caught during development).
+	 *
+	 * @param callable[] $callbacks The callbacks used to mutate the string.
+	 * @param string     $string    The string being processed.
+	 *
+	 * @return string
+	 */
+	private function apply_string_callbacks( array $callbacks, string $string ): string {
+		foreach ( $callbacks as $callback ) {
+			if ( ! is_callable( $callback ) ) {
+				_doing_it_wrong( __CLASS__ . '::apply', esc_html__( 'String processors must be an array of valid callbacks.', 'woocommerce' ), esc_html( WC()->version ) );
+				return '';
+			}
+
+			$string = (string) $callback( $string );
+		}
+
+		return $string;
+	}
+}

plugins/woocommerce/tests/php/src/Internal/Utilities/HtmlSanitizerTest.php

@@ -0,0 +1,81 @@
+<?php
+/**
+ * Tests for the HtmlSanitizer utility.
+ */
+
+use Automattic\WooCommerce\Internal\Utilities\HtmlSanitizer;
+
+/**
+ * Tests relating to HtmlSanitizer.
+ */
+class HtmlSanitizerTest extends WC_Unit_Test_Case {
+	/**
+	 * @var HtmlSanitizer
+	 */
+	private $sut;
+
+	/**
+	 * Set-up subject under test.
+	 */
+	public function set_up() {
+		$this->sut = wc_get_container()->get( HtmlSanitizer::class );
+		parent::set_up();
+	}
+
+	/**
+	 * @testdox Test inputs and outputs for the HtmlSanitizer's pre-configured TRIMMED_BALANCED_LOW_HTML_NO_LINKS rule.
+	 * @dataProvider expectations_for_trimmed_balanced_low_html_no_links_rule
+	 *
+	 * @param string $test_string The string to be sanitized.
+	 * @param string $expected    How we expect the string to look after sanitization.
+	 * @param string $explanation Notes about why we expect this/what we're testing.
+	 */
+	public function test_trimmed_balanced_low_html_sanitizer( string $test_string, string $expected, string $explanation ) {
+		$this->assertEquals( $expected, $this->sut->sanitize( $test_string, HtmlSanitizer::LOW_HTML_BALANCED_TAGS_NO_LINKS ), $explanation );
+	}
+
+	/**
+	 * Describes expectations for the HtmlSanitizer's pre-configured TRIMMED_BALANCED_LOW_HTML_NO_LINKS rule.
+	 *
+	 * @return string[][]
+	 */
+	public function expectations_for_trimmed_balanced_low_html_no_links_rule() {
+		return array(
+			array( 'Simple Text', 'Simple Text', 'Plain text without HTML tags passes through unchanged.' ),
+			array( ' Leading/trailing whitespace ', ' Leading/trailing whitespace ', 'Leading and trailing whitespace will be maintained.' ),
+			array( '<p>Paragraph</p>', '<p>Paragraph</p>', 'Paragraph tags are allowed' ),
+			array( '<div><p><i>Paragraph</i></p></div>', '<p>Paragraph</p>', 'Disallowed tags are removed, allowed tags remain.' ),
+			array( '</p> <p><img src="http://bar/icon.png" /> Purchase</p>', ' <p><img src="http://bar/icon.png" /> Purchase</p>', 'Unbalanced tags are removed.' ),
+		);
+	}
+
+	/**
+	 * @testdox Tests that 'bad' string processor callbacks are handled correctly.
+	 */
+	public function test_handling_of_invalid_processor_callbacks() {
+		$this->setExpectedIncorrectUsage( HtmlSanitizer::class . '::apply' );
+
+		$output = $this->sut->sanitize(
+			'Test string',
+			array(
+				'pre_processors' => array(
+					'strtoupper',
+					'invalid_callback_1',
+				),
+				'post_processors' => array(
+					'invalid_callback_2',
+					'strrev',
+				),
+			)
+		);
+
+		$this->assertEquals( '', $output, 'When invalid callbacks are provided, an empty string will be returned.' );
+	}
+
+	/**
+	 * @testdox An empty ruleset is equivalent to asking that all HTML elements be removed.
+	 */
+	public function test_no_kses_rules_specified() {
+		$this->assertEquals( 'foo', $this->sut->sanitize( '<p>foo</p>', array() ) );
+	}
+}