PHPCS - WordPress.Security.NonceVerification (#113)

* turn off NonceVerification

* verify nonce when ids are in params

* remove variable instance for parameter

* use core check_ajax_referer instead of inherited

* use filter_has_var to not access superglobal $_GET.

* ignore nonce verification due to limitation of phpcs

* add nonce check and modernize js

* add core nonce check

* use core nonce check

* remove unused ajax_referer on base

* early return when possible

* return early on image_sizes_notice

* handle error

* camelcase

* move pagebuilder check to picture and allow test to override behaviour

* format

* still render details when nonce is invalid

* format and typo

* format

Author: tijmenbruggeman

phpcs.xml

@@ -10,9 +10,6 @@
     <exclude name="Generic.Commenting" />
     <exclude name="Squiz.PHP.CommentedOutCode.Found" />
 
-    <!-- Fix security issues -->
-    <exclude name="WordPress.Security.NonceVerification" />
-
     <!-- Fix AlternativeFunctons-->
     <exclude name="WordPress.WP.AlternativeFunctions" />
     <exclude name="WordPress.WP.GlobalVariablesOverride.Prohibited" />

src/class-tiny-helpers.php

@@ -111,36 +111,6 @@ public static function get_mimetype( $input ) {
 	}
 
 
-	/**
-	 * Checks wether a user is viewing from a page builder
-	 *
-	 * @since 3.6.5
-	 */
-	public static function is_pagebuilder_request() {
-		$pagebuilder_keys = array(
-			'fl_builder', // Beaver Builder
-			'et_fb', // Divi Builder
-			'bricks', // Bricks Builder
-			'breakdance', // Breakdance Builder
-			'breakdance_browser', // Breakdance Builder
-			'ct_builder', // Oxygen Builder
-			'fb-edit', // Avada Live Builder
-			'builder', // Avada Live Builder
-			'spio_no_cdn', // Site Origin
-			'tatsu', // Tatsu Builder
-			'tve', // Thrive Architect
-			'tcbf', // Thrive Architect
-		);
-
-		foreach ( $pagebuilder_keys as $key ) {
-			if ( isset( $_GET[ $key ] ) ) {
-				return true;
-			}
-		}
-
-		return false;
-	}
-
 	/**
 	 * Gets or initializes the WordPress filesystem instance.
 	 *

src/class-tiny-notices.php

@@ -148,7 +148,7 @@ public function remove( $name ) {
 	}
 
 	public function dismiss() {
-		if ( empty( $_POST['name'] ) || ! $this->check_ajax_referer() ) {
+		if ( ! check_ajax_referer( 'tiny-compress', '_nonce', false ) || empty( $_POST['name'] ) ) {
 			echo json_encode( false );
 			exit();
 		}

src/class-tiny-picture.php

@@ -65,13 +65,62 @@ public function __construct( $settings, $base_dir = ABSPATH, $domains = array()
 			return;
 		}
 
-		if ( Tiny_Helpers::is_pagebuilder_request() ) {
+		if ( static::is_pagebuilder_request() ) {
 			return;
 		}
 
 		add_action( 'template_redirect', array( $this, 'on_template_redirect' ) );
 	}
 
+	/**
+	 * Checks whether the current request originates from a page builder.
+	 *
+	 * Detects known page builder query parameters to prevent picture-element
+	 * injection from interfering with builder previews.
+	 *
+	 * @since 3.6.5
+	 *
+	 * @return bool True if a page builder query parameter is present.
+	 */
+	protected static function is_pagebuilder_request() {
+		$pagebuilder_keys = array(
+			'fl_builder',        // Beaver Builder
+			'et_fb',             // Divi Builder
+			'bricks',            // Bricks Builder
+			'breakdance',        // Breakdance Builder
+			'breakdance_browser', // Breakdance Builder
+			'ct_builder',        // Oxygen Builder
+			'fb-edit',           // Avada Live Builder
+			'builder',           // Avada Live Builder
+			'spio_no_cdn',       // Site Origin
+			'tatsu',             // Tatsu Builder
+			'tve',               // Thrive Architect
+			'tcbf',              // Thrive Architect
+		);
+
+		foreach ( $pagebuilder_keys as $key ) {
+			if ( static::has_get_var( $key ) ) {
+				return true;
+			}
+		}
+
+		return false;
+	}
+
+	/**
+	 * Checks whether a GET variable exists in the original request.
+	 *
+	 * Wraps filter_has_var() to allow overriding in tests via late static binding.
+	 *
+	 * @since 3.6.5
+	 *
+	 * @param string $key The query string key to check.
+	 * @return bool True if the key exists in the original GET request.
+	 */
+	protected static function has_get_var( $key ) {
+		return filter_has_var( INPUT_GET, $key );
+	}
+
 	public function on_template_redirect() {
 		$conversion_enabled = $this->settings->get_conversion_enabled();
 		if ( apply_filters( 'tiny_replace_with_picture', $conversion_enabled ) ) {

src/class-tiny-plugin.php

@@ -517,9 +517,8 @@ public function compress_on_upload() {
 	 *               or success array ['data' => [$id, $metadata]]
 	 */
 	private function validate_ajax_attachment_request() {
-		if ( ! $this->check_ajax_referer() ) {
-			exit();
-		}
+		check_ajax_referer( 'tiny-compress', '_nonce' );
+
 		if ( ! current_user_can( 'upload_files' ) ) {
 			return array(
 				'error' => esc_html__(
@@ -614,11 +613,14 @@ public function compress_image_for_bulk() {
 		);
 		wp_update_attachment_metadata( $id, $tiny_image->get_wp_metadata() );
 
+		// Nonce verified in validate_ajax_attachment_request().
+		// phpcs:disable WordPress.Security.NonceVerification.Missing
 		$current_library_size = isset( $_POST['current_size'] ) ?
 			intval( wp_unslash( $_POST['current_size'] ) )
 			: 0;
-		$size_after           = $image_statistics['compressed_total_size'];
-		$new_library_size     = $current_library_size + $size_after - $size_before;
+		// phpcs:enable WordPress.Security.NonceVerification.Missing
+		$size_after       = $image_statistics['compressed_total_size'];
+		$new_library_size = $current_library_size + $size_after - $size_before;
 
 		$result['message']                = $tiny_image->get_latest_error();
 		$result['image_sizes_compressed'] = $image_statistics['image_sizes_compressed'];
@@ -655,7 +657,8 @@ public function compress_image_for_bulk() {
 	}
 
 	public function ajax_optimization_statistics() {
-		if ( $this->check_ajax_referer() && current_user_can( 'upload_files' ) ) {
+		if ( check_ajax_referer( 'tiny-compress', '_nonce', false ) &&
+			current_user_can( 'upload_files' ) ) {
 			$stats = Tiny_Bulk_Optimization::get_optimization_statistics( $this->settings );
 			echo json_encode( $stats );
 		}
@@ -703,6 +706,7 @@ public function media_library_bulk_action() {
 		$location = 'upload.php?mode=list&ids=' . $ids;
 
 		$location = add_query_arg( 'action', $action, $location );
+		$location = add_query_arg( '_tiny_nonce', wp_create_nonce( 'tiny-bulk-ids' ), $location );
 
 		if ( ! empty( $_REQUEST['paged'] ) ) {
 			$location = add_query_arg( 'paged', absint( $_REQUEST['paged'] ), $location );
@@ -758,6 +762,18 @@ public function show_media_info() {
 	}
 
 	private function render_compress_details( $tiny_image ) {
+		$images_to_compress = array();
+
+		if ( ! empty( $_GET['ids'] ) ) {
+			$nonce = isset( $_GET['_tiny_nonce'] ) ?
+				sanitize_key( wp_unslash( $_GET['_tiny_nonce'] ) ) : '';
+
+			if ( $nonce && wp_verify_nonce( $nonce, 'tiny-bulk-ids' ) ) {
+				$request_ids        = sanitize_text_field( wp_unslash( $_GET['ids'] ) );
+				$images_to_compress = array_map( 'intval', explode( '-', $request_ids ) );
+			}
+		}
+
 		$in_progress = $tiny_image->filter_image_sizes( 'in_progress' );
 		if ( count( $in_progress ) > 0 ) {
 			include __DIR__ . '/views/compress-details-processing.php';
@@ -816,7 +832,7 @@ public function add_dashboard_widget() {
 		/*
 		This might be deduplicated with the admin script localization, but
 			the order of including scripts is sometimes different. So in that
-			case we need to make sure that the order of inclusion is correc.t */
+			case we need to make sure that the order of inclusion is correct. */
 		wp_localize_script(
 			self::NAME . '_dashboard_widget',
 			'tinyCompressDashboard',

src/class-tiny-settings.php

@@ -160,6 +160,8 @@ public function add_options_to_page() {
 	}
 
 	public function image_sizes_notice() {
+		check_ajax_referer( 'tiny-compress' );
+
 		if ( current_user_can( 'manage_options' ) ) {
 			$selected_sizes = isset( $_GET['image_sizes_selected'] ) ?
 				intval( $_GET['image_sizes_selected'] ) : 0;
@@ -827,9 +829,8 @@ public function render_pending_status() {
 	}
 
 	public function create_api_key() {
-		if ( ! $this->check_ajax_referer() ) {
-			exit;
-		}
+		check_ajax_referer( 'tiny-compress', '_nonce' );
+
 		$compressor = $this->get_compressor();
 		if ( ! current_user_can( 'manage_options' ) ) {
 			$status = (object) array(
@@ -905,9 +906,7 @@ public function create_api_key() {
 	}
 
 	public function update_api_key() {
-		if ( ! $this->check_ajax_referer() ) {
-			exit;
-		}
+		check_ajax_referer( 'tiny-compress', '_nonce' );
 
 		$key = null;
 		if ( ! current_user_can( 'manage_options' ) ) {

src/class-tiny-wp-base.php

@@ -87,10 +87,6 @@ protected function get_user_id() {
 		return get_current_user_id();
 	}
 
-	protected function check_ajax_referer() {
-		return check_ajax_referer( 'tiny-compress', '_nonce', false );
-	}
-
 	public function init() {
 	}
 

src/js/admin.js

@@ -301,7 +301,6 @@
     eventOn('click', 'button.tiny-mark-as-compressed', onClickButtonMarkAsCompressed);
 
     setPropOf('button.tiny-compress', 'disabled', null);
-    
     compressImageSelection();
     watchCompressingImages();
 
@@ -329,29 +328,33 @@
       });
     }
 
-    eventOn('click', 'input[name*=tinypng_sizes], #tinypng_resize_original_enabled', function() {
-      /* Unfortunately, we need some additional information to display
-         the correct notice. */
-      var totalSelectedSizes = jQuery('input[name*=tinypng_sizes]:checked').length;
-      var compressWr2x = propOf('#tinypng_sizes_wr2x', 'checked');
-      if (compressWr2x) {
-        totalSelectedSizes--;
-      }
+    async function refreshSizeDescriptionNotice() {
+      const totalSelectedSizes = document.querySelectorAll('input[name*=tinypng_sizes]:checked').length;
+      const compressWr2x = document.querySelector('#tinypng_sizes_wr2x')?.checked ?? false;
+      const selectedCount = compressWr2x ? totalSelectedSizes - 1 : totalSelectedSizes;
 
-      var image_count_url = ajaxurl + (ajaxurl.indexOf( '?' ) > 0 ? '&' : '?') + 'action=tiny_image_sizes_notice&image_sizes_selected=' + totalSelectedSizes;
-      if (propOf('#tinypng_resize_original_enabled', 'checked') && propOf('#tinypng_sizes_0', 'checked')) {
-        image_count_url += '&resize_original=true';
+      const separator = ajaxurl.includes('?') ? '&' : '?';
+      let imageCountUrl = `${ajaxurl}${separator}action=tiny_image_sizes_notice&image_sizes_selected=${selectedCount}&_ajax_nonce=${tinyCompress.nonce}`;
+
+      const resizeOriginalChecked = document.querySelector('#tinypng_resize_original_enabled')?.checked;
+      const compressOriginalChecked = document.querySelector('#tinypng_sizes_0')?.checked;
+      if (resizeOriginalChecked && compressOriginalChecked) {
+        imageCountUrl += '&resize_original=true';
       }
       if (compressWr2x) {
-        image_count_url += '&compress_wr2x=true';
+        imageCountUrl += '&compress_wr2x=true';
       }
-      jQuery('#tiny-image-sizes-notice').load(image_count_url);
-    });
-
-    eventOn('click', '#tinypng_auto_compress_enabled', function() {
-      updateSettings();
-    });
+      try {
+        const sizeDescriptionHtml = await fetch(imageCountUrl).then(r => r.text());
+        document.querySelector('#tiny-image-sizes-notice').innerHTML = sizeDescriptionHtml;
+      } catch (err) {
+        document.querySelector('#tiny-image-sizes-notice').innerHTML = '';
+        console.error(err);
+      }
+    }
 
+    eventOn('click', 'input[name*=tinypng_sizes], #tinypng_resize_original_enabled', refreshSizeDescriptionNotice);
+    eventOn('click', '#tinypng_auto_compress_enabled', updateSettings);
     jQuery('#tinypng_sizes_0, #tinypng_resize_original_enabled').click(updateSettings);
     updateSettings();
   }

src/views/compress-details.php

@@ -2,8 +2,9 @@
 /**
  * Compression details on media overview page.
  *
- * @var Tiny_Plugin $this       The plugin instance.
- * @var Tiny_Image  $tiny_image The image being compressed.
+ * @var Tiny_Plugin $this               The plugin instance.
+ * @var Tiny_Image  $tiny_image         The image being compressed.
+ * @var int[]       $images_to_compress The IDs that are being compressed
  */
 
 $available_sizes              = array_keys( $this->settings->get_sizes() );
@@ -22,11 +23,6 @@
 $size_exists = array_fill_keys( $available_sizes, true );
 ksort( $size_exists );
 
-$images_to_compress = array();
-if ( ! empty( $_REQUEST['ids'] ) ) {
-	$request_ids        = sanitize_text_field( wp_unslash( $_REQUEST['ids'] ) );
-	$images_to_compress = array_map( 'intval', explode( '-', $request_ids ) );
-}
 ?>
 <div class="details-container">
 	<div class="details">

test/unit/TinyHelpersTest.php

@@ -111,26 +111,6 @@ public function test_uppercase_extension_and_mimetype_case_insensitive()
     $this->assertEquals($expected, Tiny_Helpers::replace_file_extension('image/avif', $input));
 }
 
-public function test_is_pagebuilder_request_returns_false_when_no_pagebuilder_keys()
-{
-    $_GET = array();
-    $this->assertFalse(Tiny_Helpers::is_pagebuilder_request());
-}
-
-public function test_is_pagebuilder_request_returns_true_for_beaver_builder()
-{
-    $_GET = array('fl_builder' => '1');
-    $this->assertTrue(Tiny_Helpers::is_pagebuilder_request());
-    $_GET = array();
-}
-
-public function test_is_pagebuilder_request_returns_false_for_non_pagebuilder_keys()
-{
-    $_GET = array('page' => 'settings', 'post_id' => '123');
-    $this->assertFalse(Tiny_Helpers::is_pagebuilder_request());
-    $_GET = array();
-}
-
 public function test_str_starts_with_returns_true_when_haystack_starts_with_needle()
 {
     $this->assertTrue(Tiny_Helpers::str_starts_with('hello world', 'hello'));