Changeset 42000

Timestamp:

10/24/2017 09:04:50 PM (7 years ago)

Author:

joehoyle

Message:

REST API: Don’t remove unregistered properties from objects in schema.

In r41727 the ability to sanitise and validate objects from JSON schema was added, with a whitelist approach. It was decided we should pass through all non-registered properties to reflect the behaviour of the root object in register_rest_route. To prevent arbitrary extra data via setting objects, we force additionalProperties to false in the settings endpoint.

See #38583.

Location:

trunk

Files:

• src/wp-includes/rest-api.php (modified) (2 diffs)
• src/wp-includes/rest-api/endpoints/class-wp-rest-controller.php (modified) (1 diff)
• src/wp-includes/rest-api/endpoints/class-wp-rest-settings-controller.php (modified) (2 diffs)
• tests/phpunit/tests/rest-api/rest-schema-sanitization.php (modified) (2 diffs)
• tests/phpunit/tests/rest-api/rest-schema-validation.php (modified) (1 diff)
• tests/phpunit/tests/rest-api/rest-settings-controller.php (modified) (4 diffs)

trunk/src/wp-includes/rest-api.php

@@ -1107 +1107 @@
 
         foreach ( $value as $property => $v ) {
-            if ( ! isset( $args['properties'][ $property ] ) ) {
-                continue;
-            }
-            $is_valid = rest_validate_value_from_schema( $v, $args['properties'][ $property ], $param . '[' . $property . ']' );
-
-            if ( is_wp_error( $is_valid ) ) {
-                return $is_valid;
+            if ( isset( $args['properties'][ $property ] ) ) {
+                $is_valid = rest_validate_value_from_schema( $v, $args['properties'][ $property ], $param . '[' . $property . ']' );
+                if ( is_wp_error( $is_valid ) ) {
+                    return $is_valid;
+                }
+            } elseif ( isset( $args['additionalProperties'] ) && false === $args['additionalProperties'] ) {
+                return new WP_Error( 'rest_invalid_param', sprintf( __( '%1$s is not a valid property of Object.' ), $property ) );
             }
         }
@@ -1247 +1247 @@
 
         foreach ( $value as $property => $v ) {
-            if ( ! isset( $args['properties'][ $property ] ) ) {
+            if ( isset( $args['properties'][ $property ] ) ) {
+                $value[ $property ] = rest_sanitize_value_from_schema( $v, $args['properties'][ $property ] );
+            } elseif ( isset( $args['additionalProperties'] ) && false === $args['additionalProperties'] ) {
                 unset( $value[ $property ] );
-                continue;
             }
-            $value[ $property ] = rest_sanitize_value_from_schema( $v, $args['properties'][ $property ] );
         }
 

trunk/src/wp-includes/rest-api/endpoints/class-wp-rest-controller.php

@@ -546 +546 @@
             }
 
-            foreach ( array( 'type', 'format', 'enum', 'items', 'properties' ) as $schema_prop ) {
+            foreach ( array( 'type', 'format', 'enum', 'items', 'properties', 'additionalProperties' ) as $schema_prop ) {
                 if ( isset( $params[ $schema_prop ] ) ) {
                     $endpoint_args[ $field_id ][ $schema_prop ] = $params[ $schema_prop ];

trunk/src/wp-includes/rest-api/endpoints/class-wp-rest-settings-controller.php

@@ -249 +249 @@
             }
 
+            $rest_args['schema'] = $this->set_additional_properties_to_false( $rest_args['schema'] );
+
             $rest_options[ $rest_args['name'] ] = $rest_args;
         }
@@ -302 +304 @@
         return rest_parse_request_arg( $value, $request, $param );
     }
+
+    /**
+     * Recursively add additionalProperties = false to all objects in a schema.
+     *
+     * This is need to restrict properties of objects in settings values to only
+     * registered items, as the REST API will allow additional properties by
+     * default.
+     *
+     * @since 4.9.0
+     *
+     * @param array $schema The schema array.
+     * @return array
+     */
+    protected function set_additional_properties_to_false( $schema ) {
+        switch ( $schema['type'] ) {
+            case 'object':
+                foreach ( $schema['properties'] as $key => $child_schema ) {
+                    $schema['properties'][ $key ] = $this->set_additional_properties_to_false( $child_schema );
+                }
+                $schema['additionalProperties'] = false;
+                break;
+            case 'array':
+                $schema['items'] = $this->set_additional_properties_to_false( $schema['items'] );
+                break;
+        }
+
+        return $schema;
+    }
 }

trunk/tests/phpunit/tests/rest-api/rest-schema-sanitization.php

@@ -158 +158 @@
         $this->assertEquals( array( 'a' => 1 ), rest_sanitize_value_from_schema( array( 'a' => 1 ), $schema ) );
         $this->assertEquals( array( 'a' => 1 ), rest_sanitize_value_from_schema( array( 'a' => '1' ), $schema ) );
+        $this->assertEquals( array( 'a' => 1, 'b' => 1 ), rest_sanitize_value_from_schema( array( 'a' => '1', 'b' => 1 ), $schema ) );
+    }
+
+    public function test_type_object_strips_additional_properties() {
+        $schema = array(
+            'type'       => 'object',
+            'properties' => array(
+                'a' => array(
+                    'type' => 'number',
+                ),
+            ),
+            'additionalProperties' => false,
+        );
+        $this->assertEquals( array( 'a' => 1 ), rest_sanitize_value_from_schema( array( 'a' => 1 ), $schema ) );
+        $this->assertEquals( array( 'a' => 1 ), rest_sanitize_value_from_schema( array( 'a' => '1' ), $schema ) );
+        $this->assertEquals( array( 'a' => 1 ), rest_sanitize_value_from_schema( array( 'a' => '1', 'b' => 1 ), $schema ) );
     }
 
@@ -196 +212 @@
                     'b' => 1,
                     'c' => 3,
-                ),
+                    'd' => '1',
+                ),
+                'b' => 1,
             ),
             rest_sanitize_value_from_schema(

trunk/tests/phpunit/tests/rest-api/rest-schema-validation.php

@@ -187 +187 @@
             'properties' => array(
                 'a' => array(
-                    'type' => 'number'
+                    'type' => 'number',
                 ),
             ),
         );
         $this->assertTrue( rest_validate_value_from_schema( array( 'a' => 1 ), $schema ) );
+        $this->assertTrue( rest_validate_value_from_schema( array( 'a' => 1, 'b' => 2 ), $schema ) );
         $this->assertWPError( rest_validate_value_from_schema( array( 'a' => 'invalid' ), $schema ) );
+    }
+
+    public function test_type_object_additional_properties_false() {
+        $schema = array(
+            'type'       => 'object',
+            'properties' => array(
+                'a' => array(
+                    'type' => 'number',
+                ),
+            ),
+            'additionalProperties' => false,
+        );
+        $this->assertTrue( rest_validate_value_from_schema( array( 'a' => 1 ), $schema ) );
+        $this->assertWPError( rest_validate_value_from_schema( array( 'a' => 1, 'b' => 2 ), $schema ) );
     }
 

trunk/tests/phpunit/tests/rest-api/rest-settings-controller.php

@@ -191 +191 @@
         ) );
 
+        // We have to re-register the route, as the args changes based off registered settings.
+        $this->server->override_by_default = true;
+        $this->endpoint->register_routes();
+
         // Object is cast to correct types.
         update_option( 'mycustomsetting', array( 'a' => '1' ) );
@@ -210 +214 @@
         $response = $this->server->dispatch( $request );
         $data = $response->get_data();
-        $this->assertEquals( array( 'a' => 1 ), $data['mycustomsetting'] );
+        $this->assertEquals( null, $data['mycustomsetting'] );
 
         unregister_setting( 'somegroup', 'mycustomsetting' );
@@ -373 +377 @@
     }
 
+    public function test_update_item_with_nested_object() {
+        register_setting( 'somegroup', 'mycustomsetting', array(
+            'show_in_rest' => array(
+                'schema' => array(
+                    'type'       => 'object',
+                    'properties' => array(
+                        'a' => array(
+                            'type' => 'object',
+                            'properties' => array(
+                                'b' => array(
+                                    'type' => 'number',
+                                ),
+                            ),
+                        ),
+                    ),
+                ),
+            ),
+            'type'         => 'object',
+        ) );
+
+        // We have to re-register the route, as the args changes based off registered settings.
+        $this->server->override_by_default = true;
+        $this->endpoint->register_routes();
+        wp_set_current_user( self::$administrator );
+
+        $request = new WP_REST_Request( 'PUT', '/wp/v2/settings' );
+        $request->set_param( 'mycustomsetting', array( 'a' => array( 'b' => 1, 'c' => 1 ) ) );
+        $response = $this->server->dispatch( $request );
+        $this->assertErrorResponse( 'rest_invalid_param', $response, 400 );
+    }
+
     public function test_update_item_with_object() {
         register_setting( 'somegroup', 'mycustomsetting', array(
@@ -408 +443 @@
         $this->assertEquals( array(), get_option( 'mycustomsetting' ) );
 
+        // Provide more keys.
+        $request = new WP_REST_Request( 'PUT', '/wp/v2/settings' );
+        $request->set_param( 'mycustomsetting', array( 'a' => 1, 'b' => 2 ) );
+        $response = $this->server->dispatch( $request );
+
+        $this->assertErrorResponse( 'rest_invalid_param', $response, 400 );
+
         // Setting an invalid object.
         $request = new WP_REST_Request( 'PUT', '/wp/v2/settings' );