Merge pull request #406 from ConvertKit/escape-shortcode-block-attrib…

…utes

Escape Shortcode and Block Attributes

includes/blocks/class-convertkit-block-broadcasts.php

@@ -425,13 +425,13 @@ public function render_ajax() {
 
 		// Build attributes array.
 		$atts = array(
-			'date_format'         => sanitize_text_field( $_REQUEST['date_format'] ),
-			'limit'               => sanitize_text_field( $_REQUEST['limit'] ),
+			'date_format'         => stripslashes( sanitize_text_field( $_REQUEST['date_format'] ) ),
+			'limit'               => stripslashes( sanitize_text_field( $_REQUEST['limit'] ) ),
 			'page'                => absint( $_REQUEST['page'] ),
 			'paginate'            => absint( $_REQUEST['paginate'] ),
-			'paginate_label_next' => sanitize_text_field( $_REQUEST['paginate_label_next'] ),
-			'paginate_label_prev' => sanitize_text_field( $_REQUEST['paginate_label_prev'] ),
-			'link_color'          => sanitize_text_field( $_REQUEST['link_color'] ),
+			'paginate_label_next' => stripslashes( sanitize_text_field( $_REQUEST['paginate_label_next'] ) ),
+			'paginate_label_prev' => stripslashes( sanitize_text_field( $_REQUEST['paginate_label_prev'] ) ),
+			'link_color'          => stripslashes( sanitize_text_field( $_REQUEST['link_color'] ) ),
 		);
 
 		// Parse attributes, defining fallback defaults if required
@@ -484,7 +484,7 @@ private function build_html( $posts, $atts, $include_container = true ) {
 
 		// Include container, if required.
 		if ( $include_container ) {
-			$html = '<div class="' . esc_attr( implode( ' ', $atts['_css_classes'] ) ) . '" style="' . implode( ';', $atts['_css_styles'] ) . '" ' . $this->get_atts_as_html_data_attributes( $atts ) . '>';
+			$html = '<div class="' . implode( ' ', map_deep( $atts['_css_classes'], 'sanitize_html_class' ) ) . '" style="' . implode( ';', map_deep( $atts['_css_styles'], 'esc_attr' ) ) . '" ' . $this->get_atts_as_html_data_attributes( $atts ) . '>';
 		}
 
 		// Start list.
@@ -506,8 +506,8 @@ private function build_html( $posts, $atts, $include_container = true ) {
 
 			// Add broadcast as list item.
 			$html .= '<li class="convertkit-broadcast">
-				<time datetime="' . date_i18n( 'Y-m-d', $date_timestamp ) . '">' . date_i18n( $atts['date_format'], $date_timestamp ) . '</time>
-				<a href="' . $url . '" target="_blank" rel="nofollow noopener"' . $this->get_link_style_tag( $atts ) . '>' . $broadcast['title'] . '</a>
+				<time datetime="' . esc_attr( date_i18n( 'Y-m-d', $date_timestamp ) ) . '">' . esc_html( date_i18n( $atts['date_format'], $date_timestamp ) ) . '</time>
+				<a href="' . esc_attr( $url ) . '" target="_blank" rel="nofollow noopener"' . $this->get_link_style_tag( $atts ) . '>' . esc_html( $broadcast['title'] ) . '</a>
 			</li>';
 		}
 
@@ -563,7 +563,7 @@ private function build_html( $posts, $atts, $include_container = true ) {
 	private function get_pagination_link_prev_html( $atts, $nonce ) {
 
 		return '<a href="' . esc_attr( $this->get_pagination_link( $atts['page'] - 1, $nonce ) ) . '" title="' . esc_attr( $atts['paginate_label_prev'] ) . '" data-page="' . esc_attr( (string) ( $atts['page'] - 1 ) ) . '" data-nonce="' . esc_attr( $nonce ) . '"' . $this->get_link_style_tag( $atts ) . '>
-			' . esc_attr( $atts['paginate_label_prev'] ) . '
+			' . esc_html( $atts['paginate_label_prev'] ) . '
 		</a>';
 
 	}
@@ -581,7 +581,7 @@ private function get_pagination_link_prev_html( $atts, $nonce ) {
 	private function get_pagination_link_next_html( $atts, $nonce ) {
 
 		return '<a href="' . esc_attr( $this->get_pagination_link( $atts['page'] + 1, $nonce ) ) . '" title="' . esc_attr( $atts['paginate_label_next'] ) . '" data-page="' . esc_attr( (string) ( $atts['page'] + 1 ) ) . '" data-nonce="' . esc_attr( $nonce ) . '"' . $this->get_link_style_tag( $atts ) . '>
-			' . esc_attr( $atts['paginate_label_next'] ) . '
+			' . esc_html( $atts['paginate_label_next'] ) . '
 		</a>';
 
 	}
@@ -671,7 +671,7 @@ private function get_link_style_tag( $atts ) {
 			return '';
 		}
 
-		return ' style="color:' . $atts['link_color'] . '"';
+		return ' style="color:' . esc_attr( $atts['link_color'] ) . '"';
 
 	}
 

includes/class-convertkit-resource-forms.php

@@ -108,7 +108,7 @@ public function get_html( $id ) {
 		}
 
 		// If here, return Form <script> embed.
-		return '<script async data-uid="' . $this->resources[ $id ]['uid'] . '" src="' . $this->resources[ $id ]['embed_js'] . '"></script>';
+		return '<script async data-uid="' . esc_attr( $this->resources[ $id ]['uid'] ) . '" src="' . esc_attr( $this->resources[ $id ]['embed_js'] ) . '"></script>';
 
 	}
 

includes/class-convertkit-resource-products.php

@@ -129,10 +129,10 @@ public function get_html( $id, $button_text, $css_classes = array(), $css_styles
 		if ( $return_as_span ) {
 			$html .= '<span';
 		} else {
-			$html .= '<a href="' . $this->resources[ $id ]['url'] . '"';
+			$html .= '<a href="' . esc_attr( $this->resources[ $id ]['url'] ) . '"';
 		}
 
-		$html .= ' class="wp-block-button__link ' . esc_attr( implode( ' ', $css_classes ) ) . '" style="' . implode( ';', $css_styles ) . '" data-commerce>';
+		$html .= ' class="wp-block-button__link ' . implode( ' ', map_deep( $css_classes, 'sanitize_html_class' ) ) . '" style="' . implode( ';', map_deep( $css_styles, 'esc_attr' ) ) . '" data-commerce>';
 		$html .= esc_html( $button_text );
 
 		if ( $return_as_span ) {

readme.txt

@@ -5,7 +5,7 @@ Tags: email marketing, email newsletter, newsletter, convertkit
 Requires at least: 5.0
 Tested up to: 6.1.1
 Requires PHP: 5.6.20
-Stable tag: 2.0.4
+Stable tag: 2.0.5
 License: GPLv2 or later
 License URI: http://www.gnu.org/licenses/gpl-2.0.html
 
@@ -117,6 +117,12 @@ Full Plugin documentation can be found [here](https://help.convertkit.com/en/art
 
 == Changelog ==
 
+### 2.0.5 2022-12-15
+* Fix: Broadcasts: Strip slashes on output when pagination clicked and Broadcasts are reloaded
+* Fix: Broadcasts: Sanitize and escape HTML attributes on output
+* Fix: Forms: Escape HTML attributes on output
+* Fix: Products: Sanitize and escape HTML attributes on output
+
 ### 2.0.4 2022-12-13
 * Fix: Products: PHP warning when attempting to parse an invalid Product URL
 * Fix: Landing Pages: Catch and log when an error occurs fetching a Landing Page

tests/acceptance/broadcasts/PageBlockBroadcastsCest.php

@@ -396,6 +396,49 @@ public function testBroadcastsBlockWithHexColorParameters(AcceptanceTester $I)
 		$I->seeInSource('<div class="convertkit-broadcasts has-text-color has-background" style="color:' . $textColor . ';background-color:' . $backgroundColor . '"');
 	}
 
+	/**
+	 * Test the Broadcasts block's parameters are correctly escaped on output,
+	 * to prevent XSS.
+	 *
+	 * @since   2.0.5
+	 *
+	 * @param   AcceptanceTester $I  Tester.
+	 */
+	public function testBroadcastsBlockParameterEscaping(AcceptanceTester $I)
+	{
+		// Setup Plugin and enable debug log.
+		$I->setupConvertKitPlugin($I);
+		$I->enableDebugLog($I);
+
+		// Define a 'bad' block.  This is difficult to do in Gutenberg, but let's assume it's possible.
+		$I->havePageInDatabase(
+			[
+				'post_name'    => 'convertkit-page-broadcasts-block-parameter-escaping',
+				'post_content' => '<!-- wp:convertkit/broadcasts {"limit":1,"paginate":true,"style":{"color":{"text":"red\" onmouseover=\"alert(1)\""}}} /-->',
+			]
+		);
+
+		// Load the Page on the frontend site.
+		$I->amOnPage('/convertkit-page-broadcasts-block-parameter-escaping');
+
+		// Wait for frontend web site to load.
+		$I->waitForElementVisible('body.page-template-default');
+
+		// Check that no PHP warnings or notices were output.
+		$I->checkNoWarningsAndNoticesOnScreen($I);
+
+		// Confirm that the output is escaped.
+		$I->seeInSource('style="color:red&quot; onmouseover=&quot;alert(1)&quot;"');
+		$I->dontSeeInSource('style="color:red" onmouseover="alert(1)""');
+
+		// Test pagination.
+		$I->testBroadcastsPagination($I, 'Previous', 'Next');
+
+		// Confirm that the output is still escaped.
+		$I->seeInSource('style="color:red&quot; onmouseover=&quot;alert(1)&quot;"');
+		$I->dontSeeInSource('style="color:red" onmouseover="alert(1)""');
+	}
+
 	/**
 	 * Deactivate and reset Plugin(s) after each test, if the test passes.
 	 * We don't use _after, as this would provide a screenshot of the Plugin

tests/acceptance/broadcasts/PageShortcodeBroadcastsCest.php

@@ -445,6 +445,45 @@ public function testBroadcastsShortcodeInTextEditorWithPaginationLabelParameters
 		$I->testBroadcastsPagination($I, 'Older', 'Newer');
 	}
 
+	/**
+	 * Test the [convertkit_broadcasts] shortcode parameters are correctly escaped on output,
+	 * to prevent XSS.
+	 *
+	 * @since   2.0.5
+	 *
+	 * @param   AcceptanceTester $I  Tester.
+	 */
+	public function testBroadcastsShortcodeParameterEscaping(AcceptanceTester $I)
+	{
+		// Define a 'bad' shortcode.
+		$I->havePageInDatabase(
+			[
+				'post_name'    => 'convertkit-page-broadcasts-shortcode-parameter-escaping',
+				'post_content' => '[convertkit_broadcasts date_format="F j, Y" limit="1" paginate="1" paginate_label_prev="Previous" paginate_label_next="Next" link_color=\'red" onmouseover="alert(1)"\']',
+			]
+		);
+
+		// Load the Page on the frontend site.
+		$I->amOnPage('/convertkit-page-broadcasts-shortcode-parameter-escaping');
+
+		// Wait for frontend web site to load.
+		$I->waitForElementVisible('body.page-template-default');
+
+		// Check that no PHP warnings or notices were output.
+		$I->checkNoWarningsAndNoticesOnScreen($I);
+
+		// Confirm that the output is escaped.
+		$I->seeInSource('style="color:red" onmouseover="alert(1)""');
+		$I->dontSeeInSource('style="color:red" onmouseover="alert(1)""');
+
+		// Test pagination.
+		$I->testBroadcastsPagination($I, 'Previous', 'Next');
+
+		// Confirm that the output is still escaped.
+		$I->seeInSource('style="color:red" onmouseover="alert(1)""');
+		$I->dontSeeInSource('style="color:red" onmouseover="alert(1)""');
+	}
+
 	/**
 	 * Deactivate and reset Plugin(s) after each test, if the test passes.
 	 * We don't use _after, as this would provide a screenshot of the Plugin

tests/acceptance/products/PageBlockProductCest.php

@@ -234,6 +234,41 @@ public function testProductBlockWithHexColorParameters(AcceptanceTester $I)
 		$I->seeProductOutput($I, $_ENV['CONVERTKIT_API_PRODUCT_URL'], 'Buy my product', $textColor, $backgroundColor);
 	}
 
+	/**
+	 * Test the Product block's parameters are correctly escaped on output,
+	 * to prevent XSS.
+	 *
+	 * @since   2.0.5
+	 *
+	 * @param   AcceptanceTester $I  Tester.
+	 */
+	public function testProductBlockParameterEscaping(AcceptanceTester $I)
+	{
+		// Define a 'bad' block.  This is difficult to do in Gutenberg, but let's assume it's possible.
+		$I->havePageInDatabase(
+			[
+				'post_name'    => 'convertkit-page-product-block-parameter-escaping',
+				'post_content' => '<!-- wp:convertkit/product {"product":"' . $_ENV['CONVERTKIT_API_PRODUCT_ID'] . '","style":{"color":{"text":"red\" onmouseover=\"alert(1)\""}}} /-->',
+			]
+		);
+
+		// Load the Page on the frontend site.
+		$I->amOnPage('/convertkit-page-product-block-parameter-escaping');
+
+		// Wait for frontend web site to load.
+		$I->waitForElementVisible('body.page-template-default');
+
+		// Check that no PHP warnings or notices were output.
+		$I->checkNoWarningsAndNoticesOnScreen($I);
+
+		// Confirm that the output is escaped.
+		$I->seeInSource('style="color:red&quot; onmouseover=&quot;alert(1)&quot;"');
+		$I->dontSeeInSource('style="color:red" onmouseover="alert(1)""');
+
+		// Confirm that the ConvertKit Product is displayed.
+		$I->seeProductOutput($I, $_ENV['CONVERTKIT_API_PRODUCT_URL'], 'Buy my product');
+	}
+
 	/**
 	 * Deactivate and reset Plugin(s) after each test, if the test passes.
 	 * We don't use _after, as this would provide a screenshot of the Plugin

tests/acceptance/products/PageShortcodeProductCest.php

@@ -204,6 +204,41 @@ public function testProductShortcodeWithHexColorParameters(AcceptanceTester $I)
 		$I->seeProductOutput($I, $_ENV['CONVERTKIT_API_PRODUCT_URL'], 'Buy my product', $textColor, $backgroundColor);
 	}
 
+	/**
+	 * Test the [convertkit_product] shortcode parameters are correctly escaped on output,
+	 * to prevent XSS.
+	 *
+	 * @since   2.0.5
+	 *
+	 * @param   AcceptanceTester $I  Tester.
+	 */
+	public function testProductShortcodeParameterEscaping(AcceptanceTester $I)
+	{
+		// Define a 'bad' shortcode.
+		$I->havePageInDatabase(
+			[
+				'post_name'    => 'convertkit-page-product-shortcode-parameter-escaping',
+				'post_content' => '[convertkit_product product="' . $_ENV['CONVERTKIT_API_PRODUCT_ID'] . '" text=\'Buy my product\' text_color=\'red" onmouseover="alert(1)"\']',
+			]
+		);
+
+		// Load the Page on the frontend site.
+		$I->amOnPage('/convertkit-page-product-shortcode-parameter-escaping');
+
+		// Wait for frontend web site to load.
+		$I->waitForElementVisible('body.page-template-default');
+
+		// Check that no PHP warnings or notices were output.
+		$I->checkNoWarningsAndNoticesOnScreen($I);
+
+		// Confirm that the output is escaped.
+		$I->seeInSource('style="color:red" onmouseover="alert(1)""');
+		$I->dontSeeInSource('style="color:red" onmouseover="alert(1)""');
+
+		// Confirm that the ConvertKit Product is displayed.
+		$I->seeProductOutput($I, $_ENV['CONVERTKIT_API_PRODUCT_URL'], 'Buy my product');
+	}
+
 	/**
 	 * Deactivate and reset Plugin(s) after each test, if the test passes.
 	 * We don't use _after, as this would provide a screenshot of the Plugin

wp-convertkit.php

@@ -9,7 +9,7 @@
  * Plugin Name: ConvertKit
  * Plugin URI: https://convertkit.com/
  * Description: Quickly and easily integrate ConvertKit forms into your site.
- * Version: 2.0.4
+ * Version: 2.0.5
  * Author: ConvertKit
  * Author URI: https://convertkit.com/
  * Text Domain: convertkit
@@ -25,7 +25,7 @@
 define( 'CONVERTKIT_PLUGIN_FILE', plugin_basename( __FILE__ ) );
 define( 'CONVERTKIT_PLUGIN_URL', plugin_dir_url( __FILE__ ) );
 define( 'CONVERTKIT_PLUGIN_PATH', __DIR__ );
-define( 'CONVERTKIT_PLUGIN_VERSION', '2.0.4' );
+define( 'CONVERTKIT_PLUGIN_VERSION', '2.0.5' );
 
 // Load shared classes, if they have not been included by another ConvertKit Plugin.
 if ( ! class_exists( 'ConvertKit_API' ) ) {