Merge pull request #2406 from Codeinwp/fix/security

fix: SVG sanitization for file uploaded with 'sideload' action

inc/class-main.php

@@ -42,6 +42,7 @@ public function init() {
 		if ( ! function_exists( 'is_wpcom_vip' ) ) {
 			add_filter( 'upload_mimes', array( $this, 'allow_meme_types' ), PHP_INT_MAX ); // phpcs:ignore WordPressVIPMinimum.Hooks.RestrictedHooks.upload_mimes
 			add_filter( 'wp_handle_upload_prefilter', array( $this, 'check_svg_and_sanitize' ) );
+			add_filter( 'wp_handle_sideload_prefilter', array( $this, 'check_svg_and_sanitize' ) );
 			add_filter( 'wp_check_filetype_and_ext', array( $this, 'fix_mime_type_json_svg' ), 75, 3 );
 			add_filter( 'wp_generate_attachment_metadata', array( $this, 'generate_svg_attachment_metadata' ), PHP_INT_MAX, 2 );
 		}
@@ -398,6 +399,10 @@ public function check_svg_and_sanitize( $file ) {
 					'otter-blocks'
 				);
 			}
+
+			$path_info     = pathinfo( $file['name'] );
+			$unique_suffix = '-' . substr( md5( uniqid() ), 0, 6 );
+			$file['name']  = $path_info['filename'] . $unique_suffix . '.' . $path_info['extension'];
 		}
 
 		return $file;

tests/test-svg-upload.php

@@ -21,7 +21,7 @@ private function handle_upload( $file ) {
 
 		if ( file_exists( $tmp_path ) ) {
 			return [
-				'name'     => $file,
+				'name'     => $filename,
 				'type'     => 'image/svg+xml',
 				'tmp_name' => $tmp_path,
 				'error'    => 0,
@@ -44,9 +44,29 @@ public function test_svg_upload() {
 		// We check that no error was attached.
 		$this->assertTrue( empty( $response['error'] ) );
 
+		// Check if the filename has been changed.
+		$this->assertNotEquals( $file['name'], $response['name'] );
+
 		$contents = file_get_contents( $response['tmp_name'] );
 
 		// We check that the SVG was sanitized.
 		$this->assertTrue( strpos( $contents, '<script>' ) === false );
 	}
+
+	public function test_non_svg_upload() {
+		// Set the user as the current user.
+		wp_set_current_user( 1 );
+
+		$main = new ThemeIsle\GutenbergBlocks\Main();
+		$main->init();
+
+		$file = $this->handle_upload( __DIR__ . '/assets/test-img.png' );
+		$response = $main->check_svg_and_sanitize( $file );
+
+		// We check that no error was attached.
+		$this->assertTrue( empty( $response['error'] ) );
+
+		// The filter should not change non-svg file names.
+		$this->assertEquals( $file['name'], $response['name'] );
+	}
 }