From c2df2ad7ab341b8ecf9e5b613cf7a9aa11060b47 Mon Sep 17 00:00:00 2001 From: Tim Carr Date: Thu, 15 Dec 2022 13:41:16 +0000 Subject: [PATCH 1/5] Escape attributes when output --- .../blocks/class-convertkit-block-broadcasts.php | 16 ++++++++-------- includes/class-convertkit-resource-forms.php | 2 +- includes/class-convertkit-resource-products.php | 4 ++-- 3 files changed, 11 insertions(+), 11 deletions(-) diff --git a/includes/blocks/class-convertkit-block-broadcasts.php b/includes/blocks/class-convertkit-block-broadcasts.php index 3f930af8a..9587f4e17 100644 --- a/includes/blocks/class-convertkit-block-broadcasts.php +++ b/includes/blocks/class-convertkit-block-broadcasts.php @@ -429,8 +429,8 @@ public function render_ajax() { 'limit' => 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'] ), + 'paginate_label_next' => stripslashes( sanitize_text_field( $_REQUEST['paginate_label_next'] ) ), + 'paginate_label_prev' => stripslashes( sanitize_text_field( $_REQUEST['paginate_label_prev'] ) ), 'link_color' => sanitize_text_field( $_REQUEST['link_color'] ), ); @@ -484,7 +484,7 @@ private function build_html( $posts, $atts, $include_container = true ) { // Include container, if required. if ( $include_container ) { - $html = '
get_atts_as_html_data_attributes( $atts ) . '>'; + $html = '
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 .= '
  • - - get_link_style_tag( $atts ) . '>' . $broadcast['title'] . ' + + get_link_style_tag( $atts ) . '>' . esc_html( $broadcast['title'] ) . '
    • '; } @@ -563,7 +563,7 @@ private function build_html( $posts, $atts, $include_container = true ) { private function get_pagination_link_prev_html( $atts, $nonce ) { return 'get_link_style_tag( $atts ) . '> - ' . esc_attr( $atts['paginate_label_prev'] ) . ' + ' . esc_html( $atts['paginate_label_prev'] ) . ' '; } @@ -581,7 +581,7 @@ private function get_pagination_link_prev_html( $atts, $nonce ) { private function get_pagination_link_next_html( $atts, $nonce ) { return 'get_link_style_tag( $atts ) . '> - ' . esc_attr( $atts['paginate_label_next'] ) . ' + ' . esc_html( $atts['paginate_label_next'] ) . ' '; } @@ -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'] ) . '"'; } diff --git a/includes/class-convertkit-resource-forms.php b/includes/class-convertkit-resource-forms.php index 419bb5dae..6cc249542 100644 --- a/includes/class-convertkit-resource-forms.php +++ b/includes/class-convertkit-resource-forms.php @@ -108,7 +108,7 @@ public function get_html( $id ) { } // If here, return Form '; + return ''; } diff --git a/includes/class-convertkit-resource-products.php b/includes/class-convertkit-resource-products.php index a7a7cbf32..d6d359dbd 100644 --- a/includes/class-convertkit-resource-products.php +++ b/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 .= 'resources[ $id ]['url'] . '"'; + $html .= ''; + $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 ) { From e3eb9060696a7006bceb538709d8a771dde425fe Mon Sep 17 00:00:00 2001 From: Tim Carr Date: Thu, 15 Dec 2022 13:58:03 +0000 Subject: [PATCH 2/5] Tests: Products: Check that parameters are escaped on output for block and shortcode --- .../products/PageBlockProductCest.php | 35 +++++++++++++++++++ .../products/PageShortcodeProductCest.php | 35 +++++++++++++++++++ 2 files changed, 70 insertions(+) diff --git a/tests/acceptance/products/PageBlockProductCest.php b/tests/acceptance/products/PageBlockProductCest.php index 672e56e7b..ca20e7ea5 100644 --- a/tests/acceptance/products/PageBlockProductCest.php +++ b/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' => '', + ] + ); + + // 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" 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 diff --git a/tests/acceptance/products/PageShortcodeProductCest.php b/tests/acceptance/products/PageShortcodeProductCest.php index 18e81d9bb..11e7210fe 100644 --- a/tests/acceptance/products/PageShortcodeProductCest.php +++ b/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 From 8cd336a39485a7328aa2f137fe626654e0967ff3 Mon Sep 17 00:00:00 2001 From: Tim Carr Date: Thu, 15 Dec 2022 14:15:03 +0000 Subject: [PATCH 3/5] Tests: Broadcasts: Check that parameters are escaped on output for block and shortcode --- .../broadcasts/PageBlockBroadcastsCest.php | 43 +++++++++++++++++++ .../PageShortcodeBroadcastsCest.php | 39 +++++++++++++++++ 2 files changed, 82 insertions(+) diff --git a/tests/acceptance/broadcasts/PageBlockBroadcastsCest.php b/tests/acceptance/broadcasts/PageBlockBroadcastsCest.php index c45376c62..ffbce470c 100644 --- a/tests/acceptance/broadcasts/PageBlockBroadcastsCest.php +++ b/tests/acceptance/broadcasts/PageBlockBroadcastsCest.php @@ -396,6 +396,49 @@ public function testBroadcastsBlockWithHexColorParameters(AcceptanceTester $I) $I->seeInSource('
    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' => '', + ] + ); + + // 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" 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 diff --git a/tests/acceptance/broadcasts/PageShortcodeBroadcastsCest.php b/tests/acceptance/broadcasts/PageShortcodeBroadcastsCest.php index a23fe3042..f90fbd73e 100644 --- a/tests/acceptance/broadcasts/PageShortcodeBroadcastsCest.php +++ b/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 From 5012aec236e95266c46f1d0b29e9dafcc94df1c1 Mon Sep 17 00:00:00 2001 From: Tim Carr Date: Thu, 15 Dec 2022 14:15:18 +0000 Subject: [PATCH 4/5] Broadcasts: Strip slashes from attributes, which are added when an AJAX call is made --- includes/blocks/class-convertkit-block-broadcasts.php | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/includes/blocks/class-convertkit-block-broadcasts.php b/includes/blocks/class-convertkit-block-broadcasts.php index 9587f4e17..95b636679 100644 --- a/includes/blocks/class-convertkit-block-broadcasts.php +++ b/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' => stripslashes( sanitize_text_field( $_REQUEST['paginate_label_next'] ) ), 'paginate_label_prev' => stripslashes( sanitize_text_field( $_REQUEST['paginate_label_prev'] ) ), - 'link_color' => sanitize_text_field( $_REQUEST['link_color'] ), + 'link_color' => stripslashes( sanitize_text_field( $_REQUEST['link_color'] ) ), ); // Parse attributes, defining fallback defaults if required From e70bd6d46c920f729d2b1a0cb3a73e9e1aba17de Mon Sep 17 00:00:00 2001 From: Tim Carr Date: Thu, 15 Dec 2022 14:16:33 +0000 Subject: [PATCH 5/5] Update readme file --- readme.txt | 8 +++++++- wp-convertkit.php | 4 ++-- 2 files changed, 9 insertions(+), 3 deletions(-) diff --git a/readme.txt b/readme.txt index ef3aedc13..2f801e9cb 100755 --- a/readme.txt +++ b/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 diff --git a/wp-convertkit.php b/wp-convertkit.php index f01939427..a7e52d067 100644 --- a/wp-convertkit.php +++ b/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' ) ) {