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.

git-svn-id: https://develop.svn.wordpress.org/trunk@42000 602fd350-edb4-49c9-b593-d223f7449a82

src/wp-includes/rest-api.php

@@ -1106,13 +1106,13 @@ function rest_validate_value_from_schema( $value, $args, $param = '' ) {
 		}
 
 		foreach ( $value as $property => $v ) {
-			if ( ! isset( $args['properties'][ $property ] ) ) {
+			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 ) ) {
-			$is_valid = rest_validate_value_from_schema( $v, $args['properties'][ $property ], $param . '[' . $property . ']' );
+					return $is_valid;
-
+				}
-			if ( is_wp_error( $is_valid ) ) {
+			} elseif ( isset( $args['additionalProperties'] ) && false === $args['additionalProperties'] ) {
-				return $is_valid;
+				return new WP_Error( 'rest_invalid_param', sprintf( __( '%1$s is not a valid property of Object.' ), $property ) );
 			}
 		}
 	}
@@ -1246,11 +1246,11 @@ function rest_sanitize_value_from_schema( $value, $args ) {
 		}
 
 		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 ] );
 		}
 
 		return $value;

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

@@ -545,7 +545,7 @@ abstract class WP_REST_Controller {
 				$endpoint_args[ $field_id ]['required'] = true;
 			}
 
-			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 ];
 				}

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

@@ -248,6 +248,8 @@ class WP_REST_Settings_Controller extends WP_REST_Controller {
 				continue;
 			}
 
+			$rest_args['schema'] = $this->set_additional_properties_to_false( $rest_args['schema'] );
+
 			$rest_options[ $rest_args['name'] ] = $rest_args;
 		}
 
@@ -301,4 +303,32 @@ class WP_REST_Settings_Controller extends WP_REST_Controller {
 		}
 		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;
+	}
 }

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

@@ -157,6 +157,22 @@ class WP_Test_REST_Schema_Sanitization extends WP_UnitTestCase {
 		);
 		$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 ) );
 	}
 
 	public function test_type_object_nested() {
@@ -195,7 +211,9 @@ class WP_Test_REST_Schema_Sanitization extends WP_UnitTestCase {
 				'a' => array(
 					'b' => 1,
 					'c' => 3,
+					'd' => '1',
 				),
+				'b' => 1,
 			),
 			rest_sanitize_value_from_schema(
 				array(

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

@@ -186,14 +186,29 @@ class WP_Test_REST_Schema_Validation extends WP_UnitTestCase {
 			'type'       => 'object',
 			'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 ) );
+	}
+
 	public function test_type_object_nested() {
 		$schema = array(
 			'type' => 'object',

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

@@ -190,6 +190,10 @@ class WP_Test_REST_Settings_Controller extends WP_Test_REST_Controller_Testcase
 			'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();
+
 		// Object is cast to correct types.
 		update_option( 'mycustomsetting', array( 'a' => '1' ) );
 		$request = new WP_REST_Request( 'GET', '/wp/v2/settings' );
@@ -209,7 +213,7 @@ class WP_Test_REST_Settings_Controller extends WP_Test_REST_Controller_Testcase
 		$request = new WP_REST_Request( 'GET', '/wp/v2/settings' );
 		$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' );
 	}
@@ -372,6 +376,37 @@ class WP_Test_REST_Settings_Controller extends WP_Test_REST_Controller_Testcase
 		unregister_setting( 'somegroup', 'mycustomsetting' );
 	}
 
+	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(
 			'show_in_rest' => array(
@@ -407,6 +442,13 @@ class WP_Test_REST_Settings_Controller extends WP_Test_REST_Controller_Testcase
 		$this->assertEquals( array(), $data['mycustomsetting'] );
 		$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' );
 		$request->set_param( 'mycustomsetting', array( 'a' => 'invalid' ) );