diff --git a/phpcs.xml.dist b/phpcs.xml.dist index 9c82c7a49..d2b3495b5 100644 --- a/phpcs.xml.dist +++ b/phpcs.xml.dist @@ -31,7 +31,57 @@ - + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + */pattern-translations/includes/*.php$ + + + + @@ -79,11 +129,8 @@ - - - - - + + @@ -100,55 +147,24 @@ - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - */pattern-translations/includes/*.php$ - - - - - + + + error - + - + + + diff --git a/public_html/wp-content/plugins/pattern-creator/pattern-creator.php b/public_html/wp-content/plugins/pattern-creator/pattern-creator.php index 21b59cf49..d88bef388 100644 --- a/public_html/wp-content/plugins/pattern-creator/pattern-creator.php +++ b/public_html/wp-content/plugins/pattern-creator/pattern-creator.php @@ -337,8 +337,8 @@ function allow_reading_global_styles( $caps, $cap, $user_id, $args ) { ) { return $caps; } - $request_method = isset( $_SERVER['REQUEST_METHOD'] ) ? strtoupper( $_SERVER['REQUEST_METHOD'] ) : ''; - $request_uri = isset( $_SERVER['REQUEST_URI'] ) ? $_SERVER['REQUEST_URI'] : ''; + $request_method = isset( $_SERVER['REQUEST_METHOD'] ) ? strtoupper( sanitize_text_field( wp_unslash( $_SERVER['REQUEST_METHOD'] ) ) ) : ''; + $request_uri = isset( $_SERVER['REQUEST_URI'] ) ? esc_url_raw( wp_unslash( $_SERVER['REQUEST_URI'] ) ) : ''; if ( ( 'GET' !== $request_method && 'HEAD' !== $request_method ) || false === strpos( $request_uri, '/wp/v2/global-styles' ) diff --git a/public_html/wp-content/plugins/pattern-directory/includes/admin-patterns.php b/public_html/wp-content/plugins/pattern-directory/includes/admin-patterns.php index 9091857e2..18e508b19 100644 --- a/public_html/wp-content/plugins/pattern-directory/includes/admin-patterns.php +++ b/public_html/wp-content/plugins/pattern-directory/includes/admin-patterns.php @@ -391,11 +391,8 @@ function handle_pattern_list_table_views( WP_Query $query ) { * @return array */ function display_post_states( $post_states, $post ) { - if ( isset( $_REQUEST['post_status'] ) ) { - $post_status = $_REQUEST['post_status']; - } else { - $post_status = ''; - } + // phpcs:ignore WordPress.Security.NonceVerification.Recommended -- read-only list table filter, compared against registered statuses. + $post_status = isset( $_REQUEST['post_status'] ) ? sanitize_key( wp_unslash( $_REQUEST['post_status'] ) ) : ''; if ( $post->post_status !== $post_status && diff --git a/public_html/wp-content/plugins/pattern-directory/tests/phpunit/admin-patterns-test.php b/public_html/wp-content/plugins/pattern-directory/tests/phpunit/admin-patterns-test.php new file mode 100644 index 000000000..686fd25ac --- /dev/null +++ b/public_html/wp-content/plugins/pattern-directory/tests/phpunit/admin-patterns-test.php @@ -0,0 +1,188 @@ +post->create( + array( + 'post_type' => POST_TYPE, + 'post_status' => UNLISTED_STATUS, + ) + ); + self::$spam_pattern_id = $factory->post->create( + array( + 'post_type' => POST_TYPE, + 'post_status' => SPAM_STATUS, + ) + ); + self::$published_pattern_id = $factory->post->create( + array( + 'post_type' => POST_TYPE, + 'post_status' => 'publish', + ) + ); + } + + /** + * Isolate each test from the real request. + */ + public function set_up() { + parent::set_up(); + + $this->original_request = $_REQUEST; + $_REQUEST = array(); + } + + /** + * Restore the real request. + */ + public function tear_down() { + $_REQUEST = $this->original_request; + + parent::tear_down(); + } + + /** + * Without a status filter, unlisted and spam patterns are labelled so they stand out in the + * "All" view. + * + * @dataProvider data_flagged_statuses + * + * @param string $property Name of the fixture property holding the pattern ID. + * @param string $status The post status that should be labelled. + */ + public function test_flagged_status_is_labelled_in_unfiltered_view( $property, $status ) { + $post = get_post( self::${$property} ); + + $states = display_post_states( array(), $post ); + + $this->assertArrayHasKey( $status, $states ); + $this->assertSame( get_post_status_object( $status )->label, $states[ $status ] ); + } + + /** + * Statuses that earn an extra label. + * + * @return array[] + */ + public function data_flagged_statuses() { + return array( + 'unlisted' => array( 'unlisted_pattern_id', UNLISTED_STATUS ), + 'spam' => array( 'spam_pattern_id', SPAM_STATUS ), + ); + } + + /** + * When the list table is already filtered to that status, the label is redundant and omitted. + */ + public function test_label_is_omitted_when_already_filtering_by_that_status() { + $_REQUEST['post_status'] = UNLISTED_STATUS; + $post = get_post( self::$unlisted_pattern_id ); + + $this->assertSame( array(), display_post_states( array(), $post ) ); + } + + /** + * An ordinary published pattern never gets an extra state. + */ + public function test_published_pattern_gets_no_extra_state() { + $post = get_post( self::$published_pattern_id ); + + $this->assertSame( array(), display_post_states( array(), $post ) ); + } + + /** + * Existing states are preserved rather than replaced. + */ + public function test_existing_states_are_preserved() { + $post = get_post( self::$unlisted_pattern_id ); + + $states = display_post_states( array( 'sticky' => 'Sticky' ), $post ); + + $this->assertArrayHasKey( 'sticky', $states ); + $this->assertArrayHasKey( UNLISTED_STATUS, $states ); + } + + /** + * The `post_status` request variable runs through `sanitize_key()`, which lowercases it. A + * differently-cased filter therefore matches the post status and suppresses the label, where + * an unsanitized comparison would not. + */ + public function test_post_status_request_variable_is_sanitized() { + $_REQUEST['post_status'] = strtoupper( UNLISTED_STATUS ); + $post = get_post( self::$unlisted_pattern_id ); + + $this->assertSame( array(), display_post_states( array(), $post ) ); + } + + /** + * Sanitizing strips characters a real status can never contain, so a value carrying markup + * cannot match and the label stays. + */ + public function test_post_status_carrying_markup_does_not_match() { + $_REQUEST['post_status'] = '' . UNLISTED_STATUS; + $post = get_post( self::$unlisted_pattern_id ); + + $states = display_post_states( array(), $post ); + + $this->assertArrayHasKey( UNLISTED_STATUS, $states ); + $this->assertStringNotContainsString( '' ), + 'null byte' => array( "en_US\0" ), + 'whitespace' => array( 'en US' ), + 'empty string' => array( '' ), + 'zero string' => array( '0' ), + ); + } + + /** + * A non-string `locale` cannot satisfy the string comparison, and must not raise a type error. + */ + public function test_array_locale_is_rejected() { + $_GET['locale'] = array( 'fr_FR' ); + + $this->assertSame( 'en_US', locale( 'en_US' ) ); + } + + /** + * WordPress slashes $_GET, so the value is unslashed before it is compared against its own + * sanitized form. A value that is only valid once unslashed is therefore honoured, and what + * gets returned is the sanitized form rather than the raw input. + */ + public function test_slashed_locale_is_unslashed_before_comparison() { + $_GET['locale'] = 'fr\\_FR'; + + $this->assertSame( 'fr_FR', locale( 'en_US' ) ); + } + + /** + * `?_locale=user` is rewritten on JSON requests, so localised sites don't return untranslated + * details to authenticated users. + */ + public function test_user_locale_is_rewritten_on_json_requests() { + $_SERVER['HTTP_ACCEPT'] = 'application/json'; + $_GET['_locale'] = 'user'; + + locale( 'en_US' ); + + $this->assertSame( 'site', $_GET['_locale'] ); + } + + /** + * Outside a JSON request, `_locale` is left alone. + */ + public function test_user_locale_is_untouched_on_regular_requests() { + unset( $_SERVER['HTTP_ACCEPT'], $_SERVER['CONTENT_TYPE'] ); + $_GET['_locale'] = 'user'; + + locale( 'en_US' ); + + $this->assertSame( 'user', $_GET['_locale'] ); + } + + /** + * A `_locale` other than `user` is left alone even on a JSON request. + */ + public function test_other_locale_values_are_untouched() { + $_SERVER['HTTP_ACCEPT'] = 'application/json'; + $_GET['_locale'] = 'site'; + + locale( 'en_US' ); + + $this->assertSame( 'site', $_GET['_locale'] ); + } +} diff --git a/public_html/wp-content/plugins/pattern-translations/pattern-translations.php b/public_html/wp-content/plugins/pattern-translations/pattern-translations.php index 28439fc32..1cb0d917a 100644 --- a/public_html/wp-content/plugins/pattern-translations/pattern-translations.php +++ b/public_html/wp-content/plugins/pattern-translations/pattern-translations.php @@ -124,15 +124,20 @@ function translate_page_title( $title, $post_id = null ) { * For REST API requests, the `_locale=user` GET parameter is ignored for authenticated requests, causing the rest to default to the Site locale. */ function locale( $locale ) { + // phpcs:disable WordPress.Security.NonceVerification.Recommended -- locale negotiation on GET; nothing is persisted, and the only write is to $_GET itself for the current request. + + // phpcs:ignore WordPress.Security.ValidatedSanitizedInput.InputNotSanitized -- $raw_locale is the untouched baseline the next line is compared against. + $raw_locale = isset( $_GET['locale'] ) && is_string( $_GET['locale'] ) ? wp_unslash( $_GET['locale'] ) : ''; + $safe_locale = sanitize_locale_name( $raw_locale ); + // When being requested through api.wordpress.org, respect the query variable. if ( defined( 'WPORG_IS_API' ) && WPORG_IS_API && - ! empty( $_GET['locale'] ) && - is_string( $_GET['locale'] ) && - sanitize_locale_name( $_GET['locale'] ) === $_GET['locale'] + ! empty( $raw_locale ) && + $safe_locale === $raw_locale ) { - return $_GET['locale']; + return $safe_locale; } // Respect the site locale otherwise for rest api queries. @@ -144,6 +149,7 @@ function locale( $locale ) { ) { $_GET['_locale'] = 'site'; } + // phpcs:enable WordPress.Security.NonceVerification.Recommended return $locale; } diff --git a/public_html/wp-content/tests/phpunit/bootstrap.php b/public_html/wp-content/tests/phpunit/bootstrap.php index 1876f2e33..459066afd 100644 --- a/public_html/wp-content/tests/phpunit/bootstrap.php +++ b/public_html/wp-content/tests/phpunit/bootstrap.php @@ -41,6 +41,7 @@ function _manually_load_plugins() { require dirname( dirname( __DIR__ ) ) . '/plugins/pattern-directory/bootstrap.php'; require dirname( dirname( __DIR__ ) ) . '/plugins/pattern-creator/pattern-creator.php'; + require dirname( dirname( __DIR__ ) ) . '/plugins/pattern-translations/pattern-translations.php'; } tests_add_filter( 'muplugins_loaded', '_manually_load_plugins' ); diff --git a/public_html/wp-content/themes/wporg-pattern-directory-2024/functions.php b/public_html/wp-content/themes/wporg-pattern-directory-2024/functions.php index a9d4f79cd..63e87090d 100644 --- a/public_html/wp-content/themes/wporg-pattern-directory-2024/functions.php +++ b/public_html/wp-content/themes/wporg-pattern-directory-2024/functions.php @@ -72,8 +72,8 @@ function do_pattern_actions() { return; } - $action = isset( $_REQUEST['action'] ) ? $_REQUEST['action'] : false; - $nonce = isset( $_REQUEST['_wpnonce'] ) ? $_REQUEST['_wpnonce'] : false; + $action = isset( $_REQUEST['action'] ) ? sanitize_key( wp_unslash( $_REQUEST['action'] ) ) : false; + $nonce = isset( $_REQUEST['_wpnonce'] ) ? sanitize_text_field( wp_unslash( $_REQUEST['_wpnonce'] ) ) : false; $post_id = get_the_ID(); if ( 'draft' === $action ) { @@ -112,14 +112,17 @@ function do_pattern_actions() { return; } + $report_details = isset( $_POST['report-details'] ) ? sanitize_text_field( wp_unslash( $_POST['report-details'] ) ) : ''; + $report_reason = isset( $_POST['report-reason'] ) ? intval( $_POST['report-reason'] ) : 0; + $success = wp_insert_post( array( - 'post_type' => FLAG_POST_TYPE, - 'post_parent' => $post_id, - 'post_excerpt' => sanitize_text_field( $_POST['report-details'] ), - 'post_status' => PENDING_STATUS, - 'tax_input' => array( - 'wporg-pattern-flag-reason' => intval( $_POST['report-reason'] ), + 'post_type' => FLAG_POST_TYPE, + 'post_parent' => $post_id, + 'post_excerpt' => $report_details, + 'post_status' => PENDING_STATUS, + 'tax_input' => array( + 'wporg-pattern-flag-reason' => $report_reason, ), ) ); diff --git a/public_html/wp-content/themes/wporg-pattern-directory-2024/patterns/single-my-pattern.php b/public_html/wp-content/themes/wporg-pattern-directory-2024/patterns/single-my-pattern.php index a87940fa6..c1a52c3dc 100644 --- a/public_html/wp-content/themes/wporg-pattern-directory-2024/patterns/single-my-pattern.php +++ b/public_html/wp-content/themes/wporg-pattern-directory-2024/patterns/single-my-pattern.php @@ -5,9 +5,10 @@ * Inserter: no */ -$action_status = isset( $_GET['status'] ) ? $_GET['status'] : false; -$notice = ''; -$notice_type = 'warning'; +// phpcs:ignore WordPress.Security.NonceVerification.Recommended -- read-only notice flag set by our own redirect; nothing is written here. +$action_status = isset( $_GET['status'] ) ? sanitize_key( wp_unslash( $_GET['status'] ) ) : false; +$notice = ''; +$notice_type = 'warning'; if ( 'draft-failed' === $action_status ) { $notice = __( 'Your pattern could not be updated, please try again.', 'wporg-patterns' ); } diff --git a/public_html/wp-content/themes/wporg-pattern-directory-2024/patterns/single-pattern.php b/public_html/wp-content/themes/wporg-pattern-directory-2024/patterns/single-pattern.php index eeccf62d2..2b1bd8637 100644 --- a/public_html/wp-content/themes/wporg-pattern-directory-2024/patterns/single-pattern.php +++ b/public_html/wp-content/themes/wporg-pattern-directory-2024/patterns/single-pattern.php @@ -5,9 +5,10 @@ * Inserter: no */ -$action_status = isset( $_GET['status'] ) ? $_GET['status'] : false; -$notice = ''; -$notice_type = 'warning'; +// phpcs:ignore WordPress.Security.NonceVerification.Recommended -- read-only notice flag set by our own redirect; nothing is written here. +$action_status = isset( $_GET['status'] ) ? sanitize_key( wp_unslash( $_GET['status'] ) ) : false; +$notice = ''; +$notice_type = 'warning'; if ( 'report-failed' === $action_status ) { $notice = __( 'Your pattern report could not be saved. Please try again.', 'wporg-patterns' );