From d0cd6406f44d57eafc92c9dbd24c4fdd063a5a1b Mon Sep 17 00:00:00 2001 From: pattonwebz Date: Wed, 3 Apr 2024 15:10:44 +0100 Subject: [PATCH 1/9] Sanitize DB version for safety and remove unneeded local vars when doing it --- admin/class-update-database.php | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/admin/class-update-database.php b/admin/class-update-database.php index b48536a5..d1215bc1 100644 --- a/admin/class-update-database.php +++ b/admin/class-update-database.php @@ -69,8 +69,6 @@ public function edac_update_database() { } // Update database version option. - $option_name = 'edac_db_version'; - $new_value = EDAC_DB_VERSION; - update_option( $option_name, $new_value ); + update_option( 'edac_db_version', sanitize_text_field( EDAC_DB_VERSION ) ); } } From 1c01f3334a770c117e0796dddea0e4854dd222e4 Mon Sep 17 00:00:00 2001 From: pattonwebz Date: Wed, 3 Apr 2024 15:26:55 +0100 Subject: [PATCH 2/9] Check for constants defined before outputting --- admin/site-health/class-pro.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/admin/site-health/class-pro.php b/admin/site-health/class-pro.php index 8ce8ddbb..a5370605 100644 --- a/admin/site-health/class-pro.php +++ b/admin/site-health/class-pro.php @@ -33,11 +33,11 @@ public function get() { 'fields' => array( 'version' => array( 'label' => 'Version', - 'value' => EDACP_VERSION, + 'value' => defined( 'EDACP_VERSION' ) ? esc_html( EDACP_VERSION ) : 'Unset', ), 'database_version' => array( 'label' => 'Database Version', - 'value' => EDACP_DB_VERSION, + 'value' => defined( 'EDACP_DB_VERSION' ) ? esc_html( EDACP_DB_VERSION ) : 'Unset', ), 'license_status' => array( 'label' => 'License Status', From b84b3519b48788b865af729a95b1556f8c930ac2 Mon Sep 17 00:00:00 2001 From: pattonwebz Date: Wed, 3 Apr 2024 15:28:34 +0100 Subject: [PATCH 3/9] Add escaping to healthcheck data --- admin/site-health/class-free.php | 28 ++++++++++++++-------------- admin/site-health/class-pro.php | 16 ++++++++-------- 2 files changed, 22 insertions(+), 22 deletions(-) diff --git a/admin/site-health/class-free.php b/admin/site-health/class-free.php index a157f1f2..756c1b5e 100644 --- a/admin/site-health/class-free.php +++ b/admin/site-health/class-free.php @@ -33,59 +33,59 @@ public function get() { 'fields' => array( 'version' => array( 'label' => 'Version', - 'value' => EDAC_VERSION, + 'value' => esc_html( EDAC_VERSION ), ), 'database_version' => array( 'label' => 'Database Version', - 'value' => EDAC_DB_VERSION, + 'value' => esc_html( EDAC_DB_VERSION ), ), 'policy_page' => array( 'label' => 'Policy Page', - 'value' => ( get_option( 'edac_accessibility_policy_page' ) ? get_option( 'edac_accessibility_policy_page' ) : "Unset\n" ), + 'value' => esc_html( get_option( 'edac_accessibility_policy_page' ) ? get_option( 'edac_accessibility_policy_page' ) : "Unset\n" ), ), 'activation_date' => array( 'label' => 'Activation Date', - 'value' => get_option( 'edac_activation_date' ), + 'value' => esc_html( get_option( 'edac_activation_date' ) ), ), 'footer_statement' => array( 'label' => 'Footer Statement', - 'value' => ( get_option( 'edac_add_footer_accessibility_statement' ) ? 'Enabled' : 'Disabled' ), + 'value' => esc_html( get_option( 'edac_add_footer_accessibility_statement' ) ? 'Enabled' : 'Disabled' ), ), 'delete_data' => array( 'label' => 'Delete Data', - 'value' => ( get_option( 'edac_delete_data' ) ? 'Enabled' : 'Disabled' ), + 'value' => esc_html( get_option( 'edac_delete_data' ) ? 'Enabled' : 'Disabled' ), ), 'include_statement_link' => array( 'label' => 'Include Statement Link', - 'value' => ( get_option( 'edac_include_accessibility_statement_link' ) ? 'Enabled' : 'Disabled' ), + 'value' => esc_url( get_option( 'edac_include_accessibility_statement_link' ) ? 'Enabled' : 'Disabled' ), ), 'post_types' => array( 'label' => 'Post Types', - 'value' => ( get_option( 'edac_post_types' ) ? implode( ', ', get_option( 'edac_post_types' ) ) : 'Unset' ), + 'value' => esc_html( get_option( 'edac_post_types' ) ? implode( ', ', get_option( 'edac_post_types' ) ) : 'Unset' ), ), 'simplified_sum_position' => array( 'label' => 'Simplified Sum Position', - 'value' => get_option( 'edac_simplified_summary_position' ), + 'value' => esc_html( get_option( 'edac_simplified_summary_position' ) ), ), 'simplified_sum_prompt' => array( 'label' => 'Simplified Sum Prompt', - 'value' => get_option( 'edac_simplified_summary_prompt' ), + 'value' => esc_html( get_option( 'edac_simplified_summary_prompt' ) ), ), 'post_count' => array( 'label' => 'Post Count', - 'value' => edac_get_posts_count(), + 'value' => esc_html( edac_get_posts_count() ), ), 'error_count' => array( 'label' => 'Error Count', - 'value' => edac_get_error_count(), + 'value' => absint( edac_get_error_count() ), ), 'warning_count' => array( 'label' => 'Warning Count', - 'value' => edac_get_warning_count(), + 'value' => absint( edac_get_warning_count() ), ), 'db_table_count' => array( 'label' => 'DB Table Count', - 'value' => edac_database_table_count( 'accessibility_checker' ), + 'value' => absint( edac_database_table_count( 'accessibility_checker' ) ), ), ), ); diff --git a/admin/site-health/class-pro.php b/admin/site-health/class-pro.php index a5370605..355fe86e 100644 --- a/admin/site-health/class-pro.php +++ b/admin/site-health/class-pro.php @@ -41,35 +41,35 @@ public function get() { ), 'license_status' => array( 'label' => 'License Status', - 'value' => get_option( 'edacp_license_status' ), + 'value' => esc_html( get_option( 'edacp_license_status' ) ), ), 'authorization_username' => array( 'label' => 'Authorization Username', - 'value' => ( get_option( 'edacp_authorization_username' ) ? get_option( 'edacp_authorization_username' ) : 'Unset' ), + 'value' => esc_html( get_option( 'edacp_authorization_username' ) ? get_option( 'edacp_authorization_username' ) : 'Unset' ), ), 'authorization_password' => array( 'label' => 'Authorization Password', - 'value' => ( get_option( 'edacp_authorization_password' ) ? get_option( 'edacp_authorization_password' ) : 'Unset' ), + 'value' => esc_html( get_option( 'edacp_authorization_password' ) ? get_option( 'edacp_authorization_password' ) : 'Unset' ), ), 'scan_id' => array( 'label' => 'Scan ID', - 'value' => get_transient( 'edacp_scan_id' ), + 'value' => esc_html( get_transient( 'edacp_scan_id' ) ), ), 'scan_total' => array( 'label' => 'Scan Total', - 'value' => get_transient( 'edacp_scan_total' ), + 'value' => absint( get_transient( 'edacp_scan_total' ) ), ), 'simplified_sum_heading' => array( 'label' => 'Simplified Sum Heading', - 'value' => get_option( 'edacp_simplified_summary_heading' ), + 'value' => esc_html( get_option( 'edacp_simplified_summary_heading' ) ), ), 'ignore_permissions' => array( 'label' => 'Ignore Permissions', - 'value' => ( get_option( 'edacp_ignore_user_roles' ) ? implode( ', ', get_option( 'edacp_ignore_user_roles' ) ) : 'None' ), + 'value' => esc_html( get_option( 'edacp_ignore_user_roles' ) ? implode( ', ', get_option( 'edacp_ignore_user_roles' ) ) : 'None' ), ), 'ignores_db_table_count' => array( 'label' => 'Ignores DB Table Count', - 'value' => edac_database_table_count( 'accessibility_checker_global_ignores' ), + 'value' => absint( edac_database_table_count( 'accessibility_checker_global_ignores' ) ), ), ), ); From 61a2246aa0e0bf442a05f875819a182e920cb1a6 Mon Sep 17 00:00:00 2001 From: pattonwebz Date: Wed, 3 Apr 2024 21:23:44 +0100 Subject: [PATCH 4/9] Fix sanitize callbacks on checkboxes and use single sanitizer for them all --- includes/options-page.php | 55 +++++++++++++-------------------------- 1 file changed, 18 insertions(+), 37 deletions(-) diff --git a/includes/options-page.php b/includes/options-page.php index ece39422..373704e8 100644 --- a/includes/options-page.php +++ b/includes/options-page.php @@ -6,6 +6,7 @@ */ use EDAC\Admin\Purge_Post_Data; +use EDAC\Inc\Accessibility_Statement; /** * Check if user can ignore or can manage options @@ -179,7 +180,7 @@ function edac_register_setting() { // Register settings. register_setting( 'edac_settings', 'edac_post_types', 'edac_sanitize_post_types' ); - register_setting( 'edac_settings', 'edac_delete_data', 'edac_sanitize_delete_data' ); + register_setting( 'edac_settings', 'edac_delete_data', 'edac_sanitize_checkbox' ); register_setting( 'edac_settings', 'edac_simplified_summary_prompt', @@ -198,8 +199,8 @@ function edac_register_setting() { 'default' => 'after', ) ); - register_setting( 'edac_settings', 'edac_add_footer_accessibility_statement', 'edac_sanitize_add_footer_accessibility_statement' ); - register_setting( 'edac_settings', 'edac_include_accessibility_statement_link', 'edac_sanitize_include_accessibility_statement_link' ); + register_setting( 'edac_settings', 'edac_add_footer_accessibility_statement', 'edac_sanitize_checkbox' ); + register_setting( 'edac_settings', 'edac_include_accessibility_statement_link', 'edac_sanitize_checkbox' ); register_setting( 'edac_settings', 'edac_accessibility_policy_page', 'edac_sanitize_accessibility_policy_page' ); } @@ -282,7 +283,8 @@ function edac_simplified_summary_position_cb() { * Sanitize the text position value before being saved to database * * @param array $position Position value. - * @return array + * + * @return string */ function edac_sanitize_simplified_summary_position( $position ) { if ( in_array( $position, array( 'before', 'after', 'none' ), true ) ) { @@ -320,7 +322,8 @@ function edac_simplified_summary_prompt_cb() { * Sanitize the text position value before being saved to database * * @param array $prompt The text. - * @return array + * + * @return string */ function edac_sanitize_simplified_summary_prompt( $prompt ) { if ( in_array( $prompt, array( 'when required', 'always', 'none' ), true ) ) { @@ -438,18 +441,6 @@ function edac_add_footer_accessibility_statement_cb() { get_accessibility_statement() + ( new Accessibility_Statement() )->get_accessibility_statement() ); } @@ -538,13 +517,15 @@ function edac_delete_data_cb() { } /** - * Sanitize delete data values before being saved to database + * Sanitize checkbox values before being saved to database + * + * These are passed in as strings, but we will save them as integers. * - * @param int $option Option to sanitize. - * @return int + * @since 1.11.0 + * + * @param string $input Input to sanitize. + * @return int either 1 for checked or 0 for unchecked */ -function edac_sanitize_delete_data_cb( $option ) { - if ( 1 === $option ) { - return $option; - } +function edac_sanitize_checkbox( $input ) { + return ( isset( $input ) && '1' === $input ) ? 1 : 0; } From d00808fa470451381379766112dc8a76e1ce6ce0 Mon Sep 17 00:00:00 2001 From: pattonwebz Date: Wed, 3 Apr 2024 22:03:59 +0100 Subject: [PATCH 5/9] Add sanitization around the summary numbers before saving --- includes/classes/class-summary-generator.php | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/includes/classes/class-summary-generator.php b/includes/classes/class-summary-generator.php index 45c5e8c3..02f63306 100644 --- a/includes/classes/class-summary-generator.php +++ b/includes/classes/class-summary-generator.php @@ -321,10 +321,10 @@ private function get_readability( $summary ) { */ private function save_summary_meta_data( $summary ) { update_post_meta( $this->post_id, '_edac_summary', $summary ); - update_post_meta( $this->post_id, '_edac_summary_passed_tests', $summary['passed_tests'] ); - update_post_meta( $this->post_id, '_edac_summary_errors', $summary['errors'] ); - update_post_meta( $this->post_id, '_edac_summary_warnings', $summary['warnings'] ); - update_post_meta( $this->post_id, '_edac_summary_ignored', $summary['ignored'] ); - update_post_meta( $this->post_id, '_edac_summary_contrast_errors', $summary['contrast_errors'] ); + update_post_meta( $this->post_id, '_edac_summary_passed_tests', absint( $summary['passed_tests'] ) ); + update_post_meta( $this->post_id, '_edac_summary_errors', absint( $summary['errors'] ) ); + update_post_meta( $this->post_id, '_edac_summary_warnings', absint( $summary['warnings'] ) ); + update_post_meta( $this->post_id, '_edac_summary_ignored', absint( $summary['ignored'] ) ); + update_post_meta( $this->post_id, '_edac_summary_contrast_errors', absint( $summary['contrast_errors'] ) ); } } From e2ec864f23ed1a4a19eff6a37791cd6f77117c97 Mon Sep 17 00:00:00 2001 From: pattonwebz Date: Thu, 4 Apr 2024 15:15:50 +0100 Subject: [PATCH 6/9] Sanitize density data as ints only before saving --- includes/validate.php | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/includes/validate.php b/includes/validate.php index fd4167cf..473a7966 100644 --- a/includes/validate.php +++ b/includes/validate.php @@ -282,6 +282,7 @@ function edac_get_content( $post ) { if ( isset( $parsed_url['host'] ) ) { $is_local_loopback = Helpers::is_domain_loopback( $parsed_url['host'] ); + // can only be bool. update_option( 'edac_local_loopback', $is_local_loopback ); } } @@ -356,7 +357,11 @@ function edac_get_content( $post ) { $body_density_data = edac_get_body_density_data( $page_html ); if ( false !== $body_density_data ) { - update_post_meta( $post->ID, '_edac_density_data', $body_density_data ); + update_post_meta( + $post->ID, + '_edac_density_data', + array_map( 'intval', $body_density_data ) + ); } else { delete_post_meta( $post->ID, '_edac_density_data' ); } From af883178941527577bee21a01a46f8a0a184c5ff Mon Sep 17 00:00:00 2001 From: pattonwebz Date: Thu, 4 Apr 2024 15:18:48 +0100 Subject: [PATCH 7/9] Sanitize issue_density float before saving --- includes/classes/class-summary-generator.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/includes/classes/class-summary-generator.php b/includes/classes/class-summary-generator.php index 02f63306..447654ee 100644 --- a/includes/classes/class-summary-generator.php +++ b/includes/classes/class-summary-generator.php @@ -264,7 +264,7 @@ private function update_issue_density( $summary ) { $content_length = $issue_density_array[0][1]; $issue_density = edac_get_issue_density( $issue_count, $element_count, $content_length ); - update_post_meta( $this->post_id, '_edac_issue_density', $issue_density ); + update_post_meta( $this->post_id, '_edac_issue_density', floatval( $issue_density ) ); } else { delete_post_meta( $this->post_id, '_edac_issue_density' ); } From 23975b98adff441174091d242566ac9d01a0da0a Mon Sep 17 00:00:00 2001 From: pattonwebz Date: Thu, 4 Apr 2024 15:30:03 +0100 Subject: [PATCH 8/9] Make it clear that the summary is being sanitized when storing --- admin/class-ajax.php | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/admin/class-ajax.php b/admin/class-ajax.php index 28516a4f..ecae7831 100644 --- a/admin/class-ajax.php +++ b/admin/class-ajax.php @@ -72,7 +72,7 @@ public function summary() { $html['password_protected'] = $notice_text; $html['content'] .= '
' . $notice_text . '
'; } - + $post_id = (int) $_REQUEST['post_id']; $summary = ( new Summary_Generator( $post_id ) )->generate_summary(); $simplified_summary_text = ''; @@ -627,8 +627,8 @@ public function add_ignore() { global $wpdb; $table_name = $wpdb->prefix . 'accessibility_checker'; $raw_ids = isset( $_REQUEST['ids'] ) ? $_REQUEST['ids'] : array(); // phpcs:ignore WordPress.Security.ValidatedSanitizedInput.InputNotSanitized -- Sanitization handled below. - $ids = array_map( - function ( $value ) { + $ids = array_map( + function ( $value ) { return (int) $value; }, $raw_ids @@ -701,9 +701,11 @@ public function simplified_summary() { } $post_id = (int) $_REQUEST['post_id']; - $summary = sanitize_text_field( $_REQUEST['summary'] ); - - update_post_meta( $post_id, '_edac_simplified_summary', $summary ); + update_post_meta( + $post_id, + '_edac_simplified_summary', + sanitize_text_field( $_REQUEST['summary'] ) + ); $edac_simplified_summary = get_post_meta( $post_id, '_edac_simplified_summary', $single = true ); $simplified_summary = $edac_simplified_summary ? $edac_simplified_summary : ''; From 5d8dae380b7e37bb5931f4eadbd8e407b38e6ff7 Mon Sep 17 00:00:00 2001 From: pattonwebz Date: Wed, 3 Apr 2024 22:26:39 +0100 Subject: [PATCH 9/9] Add a sanitizer for the summary_meta_data --- includes/classes/class-summary-generator.php | 24 +++++++++++++++++++- 1 file changed, 23 insertions(+), 1 deletion(-) diff --git a/includes/classes/class-summary-generator.php b/includes/classes/class-summary-generator.php index 447654ee..982808b2 100644 --- a/includes/classes/class-summary-generator.php +++ b/includes/classes/class-summary-generator.php @@ -320,11 +320,33 @@ private function get_readability( $summary ) { * @since 1.9.0 */ private function save_summary_meta_data( $summary ) { - update_post_meta( $this->post_id, '_edac_summary', $summary ); + update_post_meta( $this->post_id, '_edac_summary', $this->sanitize_summary_meta_data( $summary ) ); update_post_meta( $this->post_id, '_edac_summary_passed_tests', absint( $summary['passed_tests'] ) ); update_post_meta( $this->post_id, '_edac_summary_errors', absint( $summary['errors'] ) ); update_post_meta( $this->post_id, '_edac_summary_warnings', absint( $summary['warnings'] ) ); update_post_meta( $this->post_id, '_edac_summary_ignored', absint( $summary['ignored'] ) ); update_post_meta( $this->post_id, '_edac_summary_contrast_errors', absint( $summary['contrast_errors'] ) ); } + + /** + * Sanitizes the summary metadata before saving it to the database. + * + * @param array $summary An associative array containing the summary of accessibility checks. + * + * @return array The sanitized summary metadata. + * + * @since 1.11.0 + */ + private function sanitize_summary_meta_data( array $summary ): array { + return array( + 'passed_tests' => absint( $summary['passed_tests'] ?? 0 ), + 'errors' => absint( $summary['errors'] ?? 0 ), + 'warnings' => absint( $summary['warnings'] ?? 0 ), + 'ignored' => absint( $summary['ignored'] ?? 0 ), + 'contrast_errors' => absint( $summary['contrast_errors'] ?? 0 ), + 'content_grade' => absint( $summary['content_grade'] ?? 0 ), + 'readability' => sanitize_text_field( $summary['readability'] ?? '' ), + 'simplified_summary' => filter_var( $summary['simplified_summary'] ?? false, FILTER_VALIDATE_BOOLEAN ), + ); + } }