Changeset 49753 for branches/5.6

Timestamp:

12/04/2020 09:44:27 PM (6 years ago)

Author:

iandunn

Message:

Media: Return WP_Error when cropping with bad input to avoid fatal.

This avoids an error on PHP 8 caused by calling wp_imagecreatetruecolor() with inputs that aren't numeric, or are less than 0.

Props hellofromtonya, Boniu91, metalandcoffee, SergeyBiryukov.
Reviewed by SergeyBiryukov, iandunn.
Merges [49751] to the 5.6 branch.
Fixes #51937.

Location:

branches/5.6

Files:

• . (modified) (1 prop)
• src/wp-includes/class-wp-image-editor-gd.php (modified) (2 diffs)
• src/wp-includes/media.php (modified) (1 diff)
• tests/phpunit/tests/image/editorGd.php (modified) (2 diffs)

branches/5.6/src/wp-includes/class-wp-image-editor-gd.php

@@ -324 +324 @@
                 }
 
-                $dst = wp_imagecreatetruecolor( $dst_w, $dst_h );
+                foreach ( array( $src_w, $src_h, $dst_w, $dst_h ) as $value ) {
+                        if ( ! is_numeric( $value ) || (int) $value <= 0 ) {
+                                return new WP_Error( 'image_crop_error', __( 'Image crop failed.' ), $this->file );
+                        }
+                }
+
+                $dst = wp_imagecreatetruecolor( (int) $dst_w, (int) $dst_h );
 
                 if ( $src_abs ) {
@@ -335 +341 @@
                 }
 
-                imagecopyresampled( $dst, $this->image, 0, 0, $src_x, $src_y, $dst_w, $dst_h, $src_w, $src_h );
+                imagecopyresampled( $dst, $this->image, 0, 0, (int) $src_x, (int) $src_y, (int) $dst_w, (int) $dst_h, (int) $src_w, (int) $src_h );
 
                 if ( is_gd_image( $dst ) ) {

branches/5.6/src/wp-includes/media.php

@@ -3506 +3506 @@
  * @param int $width  Image width in pixels.
  * @param int $height Image height in pixels.
- * @return resource|GdImage The GD image resource or GdImage instance.
+ * @return resource|GdImage|false The GD image resource or GdImage instance on success. False on failure.
  */
 function wp_imagecreatetruecolor( $width, $height ) {

branches/5.6/tests/phpunit/tests/image/editorGd.php

@@ -406 +406 @@
 
         /**
-         * Test cropping an image
-         */
-        public function test_crop() {
+         * Test cropping an image.
+         *
+         * @ticket 51937
+         *
+         * @dataProvider data_crop
+         */
+        public function test_crop( $src_x, $src_y, $src_w, $src_h, $dst_w = null, $dst_h = null, $src_abs = false ) {
                 $file = DIR_TESTDATA . '/images/gradient-square.jpg';
 
@@ -414 +418 @@
                 $gd_image_editor->load();
 
-                $gd_image_editor->crop( 0, 0, 50, 50 );
+                $gd_image_editor->crop( $src_x, $src_y, $src_w, $src_h, $dst_w, $dst_h, $src_abs );
 
                 $this->assertSame(
                         array(
-                                'width'  => 50,
-                                'height' => 50,
+                                'width'  => (int) $src_w,
+                                'height' => (int) $src_h,
                         ),
                         $gd_image_editor->get_size()
+                );
+        }
+
+        public function data_crop() {
+                return array(
+                        'src height and width must be greater than 0' => array(
+                                'src_x' => 0,
+                                'src_y' => 0,
+                                'src_w' => 50,
+                                'src_h' => 50,
+                        ),
+                        'src height and width can be string but must be greater than 0' => array(
+                                'src_x' => 10,
+                                'src_y' => '10',
+                                'src_w' => '50',
+                                'src_h' => '50',
+                        ),
+                        'dst height and width must be greater than 0' => array(
+                                'src_x' => 10,
+                                'src_y' => '10',
+                                'src_w' => 150,
+                                'src_h' => 150,
+                                'dst_w' => 150,
+                                'dst_h' => 150,
+                        ),
+                        'dst height and width can be string but must be greater than 0' => array(
+                                'src_x' => 10,
+                                'src_y' => '10',
+                                'src_w' => 150,
+                                'src_h' => 150,
+                                'dst_w' => '150',
+                                'dst_h' => '150',
+                        ),
+                );
+        }
+
+        /**
+         * Test should return WP_Error when dimensions are not integer or are <= 0.
+         *
+         * @ticket 51937
+         *
+         * @dataProvider data_crop_invalid_dimensions
+         */
+        public function test_crop_invalid_dimensions( $src_x, $src_y, $src_w, $src_h, $dst_w = null, $dst_h = null, $src_abs = false ) {
+                $file = DIR_TESTDATA . '/images/gradient-square.jpg';
+
+                $gd_image_editor = new WP_Image_Editor_GD( $file );
+                $gd_image_editor->load();
+
+                $actual = $gd_image_editor->crop( $src_x, $src_y, $src_w, $src_h, $dst_w, $dst_h, $src_abs );
+
+                $this->assertInstanceOf( 'WP_Error', $actual );
+                $this->assertSame( 'image_crop_error', $actual->get_error_code() );
+        }
+
+        public function data_crop_invalid_dimensions() {
+                return array(
+                        'src height must be greater than 0' => array(
+                                'src_x' => 0,
+                                'src_y' => 0,
+                                'src_w' => 100,
+                                'src_h' => 0,
+                        ),
+                        'src width must be greater than 0'  => array(
+                                'src_x' => 10,
+                                'src_y' => '10',
+                                'src_w' => 0,
+                                'src_h' => 100,
+                        ),
+                        'src height must be numeric and greater than 0' => array(
+                                'src_x' => 10,
+                                'src_y' => '10',
+                                'src_w' => 100,
+                                'src_h' => 'NaN',
+                        ),
+                        'dst height must be numeric and greater than 0' => array(
+                                'src_x' => 0,
+                                'src_y' => 0,
+                                'src_w' => 100,
+                                'src_h' => 50,
+                                'dst_w' => '100',
+                                'dst_h' => 'NaN',
+                        ),
+                        'src and dst height and width must be greater than 0' => array(
+                                'src_x' => 0,
+                                'src_y' => 0,
+                                'src_w' => 0,
+                                'src_h' => 0,
+                                'dst_w' => 0,
+                                'dst_h' => 0,
+                        ),
+                        'src and dst height and width can be string but must be greater than 0' => array(
+                                'src_x' => 0,
+                                'src_y' => 0,
+                                'src_w' => '0',
+                                'src_h' => '0',
+                                'dst_w' => '0',
+                                'dst_h' => '0',
+                        ),
                 );
         }