diff --git a/projects/packages/connection/changelog/update-connection-processing-whole-input-comments b/projects/packages/connection/changelog/update-connection-processing-whole-input-comments new file mode 100644 index 000000000000..326946ab7032 --- /dev/null +++ b/projects/packages/connection/changelog/update-connection-processing-whole-input-comments @@ -0,0 +1,4 @@ +Significance: patch +Type: deprecated + +Jetpack Connection Manager: Deprecate `request_params` arg in setup_xmlrpc_handlers method diff --git a/projects/packages/connection/legacy/class-jetpack-signature.php b/projects/packages/connection/legacy/class-jetpack-signature.php index d4e48690ad2d..58e415ce6d72 100644 --- a/projects/packages/connection/legacy/class-jetpack-signature.php +++ b/projects/packages/connection/legacy/class-jetpack-signature.php @@ -93,9 +93,9 @@ public function sign_current_request( $override = array() ) { // Convert the $_POST to the body, if the body was empty. This is how arrays are hashed // and encoded on the Jetpack side. if ( defined( 'IS_WPCOM' ) && IS_WPCOM ) { - // phpcs:ignore WordPress.Security.NonceVerification.Missing + // phpcs:ignore WordPress.Security.NonceVerification.Missing -- Used to generate a cryptographic signature of the post data. if ( empty( $body ) && is_array( $_POST ) && $_POST !== array() ) { - $body = $_POST; // phpcs:ignore WordPress.Security.NonceVerification.Missing + $body = $_POST; // phpcs:ignore WordPress.Security.NonceVerification.Missing -- We need all of $_POST in order to generate a cryptographic signature of the post data. } } } elseif ( isset( $_SERVER['REQUEST_METHOD'] ) && 'PUT' === strtoupper( $_SERVER['REQUEST_METHOD'] ) ) { // phpcs:ignore WordPress.Security.ValidatedSanitizedInput -- This is validating. diff --git a/projects/packages/connection/src/class-authorize-json-api.php b/projects/packages/connection/src/class-authorize-json-api.php index 16eca0efa07a..08b9aa63a4e1 100644 --- a/projects/packages/connection/src/class-authorize-json-api.php +++ b/projects/packages/connection/src/class-authorize-json-api.php @@ -40,9 +40,17 @@ class Authorize_Json_Api { * @return void */ public function verify_json_api_authorization_request( $environment = null ) { - $environment = $environment === null - ? $_REQUEST // phpcs:ignore WordPress.Security.NonceVerification.Recommended -- nonce verification handled later in function. - : $environment; + if ( null === $environment ) { + $request_params_needed_for_auth = array( + 'data', + 'jetpack_json_api_original_query', + 'nonce', + 'signature', + 'timestamp', + 'token', + ); + $environment = array_intersect_key( $request_params_needed_for_auth, $_REQUEST ); // phpcs:ignore WordPress.Security.NonceVerification.Recommended,WordPress.Security.ValidatedSanitizedInput.MissingUnslash,WordPress.Security.ValidatedSanitizedInput.InputNotSanitized -- nonce verification handled later in function and request data are 1) used to verify a cryptographic signature of the request data and 2) sanitized later in function. + } if ( ! isset( $environment['token'] ) ) { wp_die( esc_html__( 'You must connect your Jetpack plugin to WordPress.com to use this feature.', 'jetpack-connection' ) ); diff --git a/projects/packages/connection/src/class-manager.php b/projects/packages/connection/src/class-manager.php index fef724b52051..57ae09d846df 100644 --- a/projects/packages/connection/src/class-manager.php +++ b/projects/packages/connection/src/class-manager.php @@ -111,7 +111,7 @@ public static function configure() { ); $manager->setup_xmlrpc_handlers( - $_GET, // phpcs:ignore WordPress.Security.NonceVerification.Recommended + null, $manager->has_connected_owner(), $manager->verify_xml_rpc_signature() ); @@ -161,32 +161,31 @@ public static function configure() { * Sets up the XMLRPC request handlers. * * @since 1.25.0 Deprecate $is_active param. + * @since $$next-version$$ Deprecate $request_params param. * - * @param array $request_params incoming request parameters. + * @param array|null $deprecated Deprecated. Not used. * @param bool $has_connected_owner Whether the site has a connected owner. * @param bool $is_signed whether the signature check has been successful. * @param Jetpack_XMLRPC_Server $xmlrpc_server (optional) an instance of the server to use instead of instantiating a new one. */ public function setup_xmlrpc_handlers( - $request_params, + $deprecated, $has_connected_owner, $is_signed, Jetpack_XMLRPC_Server $xmlrpc_server = null ) { add_filter( 'xmlrpc_blog_options', array( $this, 'xmlrpc_options' ), 1000, 2 ); - - if ( - ! isset( $request_params['for'] ) - || 'jetpack' !== $request_params['for'] - ) { + if ( $deprecated !== null ) { + _deprecated_argument( __METHOD__, '$$next-version$$' ); + } + // phpcs:ignore WordPress.Security.NonceVerification.Recommended -- We are using the 'for' request param to early return unless it's 'jetpack'. + if ( ! isset( $_GET['for'] ) || 'jetpack' !== $_GET['for'] ) { return false; } // Alternate XML-RPC, via ?for=jetpack&jetpack=comms. - if ( - isset( $request_params['jetpack'] ) - && 'comms' === $request_params['jetpack'] - ) { + // phpcs:ignore WordPress.Security.NonceVerification.Recommended -- This just determines whether to handle the request as an XML-RPC request. The actual XML-RPC endpoints do the appropriate nonce checking where applicable. Plus we make sure to clear all cookies via require_jetpack_authentication called later in method. + if ( isset( $_GET['jetpack'] ) && 'comms' === $_GET['jetpack'] ) { if ( ! Constants::is_defined( 'XMLRPC_REQUEST' ) ) { // Use the real constant here for WordPress' sake. define( 'XMLRPC_REQUEST', true ); @@ -455,9 +454,9 @@ private function internal_verify_xml_rpc_signature() { } $jetpack_signature = new \Jetpack_Signature( $token->secret, (int) \Jetpack_Options::get_option( 'time_diff' ) ); - // phpcs:disable WordPress.Security.NonceVerification.Missing + // phpcs:disable WordPress.Security.NonceVerification.Missing -- Used to verify a cryptographic signature of the post data. if ( isset( $_POST['_jetpack_is_multipart'] ) ) { - $post_data = $_POST; + $post_data = $_POST; // We need all of $_POST in order to verify a cryptographic signature of the post data. $file_hashes = array(); foreach ( $post_data as $post_data_key => $post_data_value ) { if ( ! str_starts_with( $post_data_key, '_jetpack_file_hmac_' ) ) { diff --git a/projects/packages/connection/src/class-webhooks.php b/projects/packages/connection/src/class-webhooks.php index 1869a953bf0a..de580c069497 100644 --- a/projects/packages/connection/src/class-webhooks.php +++ b/projects/packages/connection/src/class-webhooks.php @@ -107,7 +107,7 @@ public function handle_authorize() { } do_action( 'jetpack_client_authorize_processing' ); - $data = stripslashes_deep( $_GET ); + $data = stripslashes_deep( $_GET ); // We need all request data under the context of an authorization request. $data['auth_type'] = 'client'; $roles = new Roles(); $role = $roles->translate_current_user_to_role(); diff --git a/projects/packages/connection/src/sso/class-user-admin.php b/projects/packages/connection/src/sso/class-user-admin.php index 8a4b66c44546..94fc775a4d98 100644 --- a/projects/packages/connection/src/sso/class-user-admin.php +++ b/projects/packages/connection/src/sso/class-user-admin.php @@ -835,7 +835,7 @@ public function render_custom_email_message_form_field( $type ) { * @return boolean Indicating if the core invitation main should be sent. */ public function should_send_wp_mail_new_user( $send_wp_email ) { - if ( ! isset( $_POST['invite_user_wpcom'] ) && isset( $_POST['send_user_notification'] ) ) { // phpcs:ignore WordPress.Security.NonceVerification.Missing + if ( ! isset( $_POST['invite_user_wpcom'] ) && isset( $_POST['send_user_notification'] ) ) { // phpcs:ignore WordPress.Security.NonceVerification.Missing -- Hooked to 'wp_send_new_user_notification_to_user' to conditionally disable the core invitation email. At this point nonces should be checked already. return $send_wp_email; } return false; diff --git a/projects/plugins/jetpack/changelog/update-connection-processing-whole-input-comments b/projects/plugins/jetpack/changelog/update-connection-processing-whole-input-comments new file mode 100644 index 000000000000..d71c36a3da34 --- /dev/null +++ b/projects/plugins/jetpack/changelog/update-connection-processing-whole-input-comments @@ -0,0 +1,5 @@ +Significance: patch +Type: other +Comment: Jetpack: Update connection related unit tests + + diff --git a/projects/plugins/jetpack/tests/php/general/test_class.jetpack.php b/projects/plugins/jetpack/tests/php/general/test_class.jetpack.php index b99202f86a50..3dc924f31a57 100644 --- a/projects/plugins/jetpack/tests/php/general/test_class.jetpack.php +++ b/projects/plugins/jetpack/tests/php/general/test_class.jetpack.php @@ -96,6 +96,7 @@ public function set_up() { */ public function tear_down() { parent::tear_down(); + unset( $_GET['for'] ); Constants::clear_constants(); StatusCache::clear(); } @@ -770,11 +771,13 @@ public static function filter_pre_update_option_jetpack_active_modules( $modules private function mocked_setup_xmlrpc_handlers( $request_params, $has_connected_owner, $is_signed, $user = false ) { $GLOBALS['HTTP_RAW_POST_DATA'] = ''; + $_GET['for'] = $request_params['for']; + Constants::set_constant( 'XMLRPC_REQUEST', true ); $jetpack = new MockJetpack(); $xmlrpc_server = new MockJetpack_XMLRPC_Server( $user ); - return $jetpack::connection()->setup_xmlrpc_handlers( $request_params, $has_connected_owner, $is_signed, $xmlrpc_server ); + return $jetpack::connection()->setup_xmlrpc_handlers( null, $has_connected_owner, $is_signed, $xmlrpc_server ); } /**