diff --git a/inc/admin.php b/inc/admin.php index d1b37e76..b1d1d291 100755 --- a/inc/admin.php +++ b/inc/admin.php @@ -222,7 +222,7 @@ public function check_svg_and_sanitize( $file ) { * * @return bool|int */ - protected function sanitize_svg( $file ) { + public function sanitize_svg( $file ) { // We can ignore the phpcs warning here as we're reading and writing to the Temp file. $dirty = file_get_contents( $file ); // phpcs:ignore @@ -253,10 +253,11 @@ protected function sanitize_svg( $file ) { $clean = gzencode( $clean ); } - // We can ignore the phpcs warning here as we're reading and writing to the Temp file. - file_put_contents( $file, $clean ); // phpcs:ignore + // We handle the write result below; silence the warning on I/O failure. Reading/writing the temp file is intended. + $written = @file_put_contents( $file, $clean ); // phpcs:ignore WordPress.WP.AlternativeFunctions, WordPress.PHP.NoSilencedErrors - return true; + // A failed write leaves the dirty upload in place, so report it as unsanitized. + return is_string( $clean ) && strlen( $clean ) === $written; } /** diff --git a/inc/dashboard_widget.php b/inc/dashboard_widget.php index 2c7756ad..097e8a8b 100644 --- a/inc/dashboard_widget.php +++ b/inc/dashboard_widget.php @@ -31,7 +31,7 @@ public function init() { * Add the dashboard widget. */ public function add_dashboard_widget() { - if ( ! $this->has_at_least_ten_visits() ) { + if ( ! current_user_can( 'manage_options' ) || ! $this->has_at_least_ten_visits() ) { return; } @@ -42,7 +42,7 @@ public function add_dashboard_widget() { * Enqueue the widget assets. */ public function enqueue_widget() { - if ( ! $this->is_main_dashboard_page() || ! $this->has_at_least_ten_visits() ) { + if ( ! current_user_can( 'manage_options' ) || ! $this->is_main_dashboard_page() || ! $this->has_at_least_ten_visits() ) { return; } @@ -112,7 +112,7 @@ private function get_script_localization() { ], 'skeletonLoader' => $this->get_skeleton_loader(), 'billingURL' => tsdk_translate_link( 'https://dashboard.optimole.com/settings/billing', 'query' ), - 'serviceData' => $this->get_service_data(), + 'serviceData' => $this->get_widget_service_data(), 'assetsURL' => OPTML_URL . 'assets/', 'dashboardMetricsURL' => esc_url( 'https://dashboard.optimole.com/metrics' ), 'dashboardURL' => esc_url( tsdk_translate_link( 'https://dashboard.optimole.com' ) ), @@ -130,6 +130,24 @@ private function get_service_data() { return $settings->get( 'service_data' ); } + /** + * Get the service data fields the widget displays, keeping keys and secrets out of the page. + * + * @return array + */ + private function get_widget_service_data() { + $service_data = $this->get_service_data(); + + if ( ! is_array( $service_data ) ) { + return []; + } + + return array_intersect_key( + $service_data, + array_flip( [ 'visitors', 'visitors_pretty', 'visitors_limit', 'visitors_limit_pretty', 'compression_percentage', 'traffic' ] ) + ); + } + /** * Get the skeleton loader markup. * diff --git a/inc/media_rename/attachment_replace.php b/inc/media_rename/attachment_replace.php index d4b19733..31424def 100644 --- a/inc/media_rename/attachment_replace.php +++ b/inc/media_rename/attachment_replace.php @@ -77,6 +77,12 @@ public function replace() { return new WP_Error( 'file_error', __( 'The uploaded file type does not match the original file type.', 'optimole-wp' ) ); } + if ( $uploaded_filetype['type'] === 'image/svg+xml' ) { + if ( ! Optml_Main::instance()->admin->sanitize_svg( $this->file['tmp_name'] ) ) { + return new WP_Error( 'file_error', __( 'Error uploading file.', 'optimole-wp' ) ); + } + } + global $wp_filesystem; if ( ! $wp_filesystem->move( $this->file['tmp_name'], $original_file, true ) ) { diff --git a/inc/v2/BgOptimizer/Lazyload.php b/inc/v2/BgOptimizer/Lazyload.php index a95dd154..73edb7fd 100644 --- a/inc/v2/BgOptimizer/Lazyload.php +++ b/inc/v2/BgOptimizer/Lazyload.php @@ -13,6 +13,25 @@ */ class Lazyload { const MARKER = '/* OPTML_VIEWPORT_BG_SELECTORS */'; + /** + * Shape of a plain selector the frontend getUniqueSelector() reports: ids, tags, classes, ' > ' and :nth-of-type(N). + */ + const SAFE_SELECTOR_PATTERN = '/^(?:[\w\-#. >\x{00A0}-\x{10FFFF}]|:nth-of-type\(\d+\))+$/u'; + + /** + * Check whether a client-reported selector is a plain selector safe to embed in the stylesheet as-is. + * + * A selector carrying CSS metacharacters (`(`, `)`, `{`, `;`, `:` beyond nth-of-type, `/`, …) is not + * embedded: the browser already discards the whole rule when it sees one, so treating it as unsafe + * keeps the current behaviour while making CSS injection impossible. + * + * @param mixed $selector The selector to check. + * + * @return bool Whether the selector matches the plain-selector shape. + */ + public static function is_safe_selector( $selector ): bool { + return is_string( $selector ) && $selector !== '' && preg_match( self::SAFE_SELECTOR_PATTERN, $selector ) === 1; + } /** * Get the current personalized CSS for lazy loading. * @@ -33,41 +52,26 @@ public static function get_personalized_css( $data ) { $lazyload_selectors = array_fill_keys( $lazyload_selectors, true ); $css_selectors = []; $preload_urls = []; + $tainted = []; foreach ( Profile::get_active_devices() as $device ) { $personalized_selectors = $data[ $device ]['bg'] ?? []; $lcp_data = $data[ $device ]['lcp'] ?? []; + // Guard against malformed profile shapes (e.g. a non-normalizing custom storage backend). + $personalized_selectors = is_array( $personalized_selectors ) ? $personalized_selectors : []; + $lcp_data = is_array( $lcp_data ) ? $lcp_data : []; if ( OPTML_DEBUG ) { do_action( 'optml_log', 'personalized_selectors: ' . $device . ' ' . print_r( $personalized_selectors, true ) ); do_action( 'optml_log', 'LCP data: ' . $device . ' ' . print_r( $lcp_data, true ) ); } - $css_selectors[ $device ] = []; - foreach ( $personalized_selectors as $selector => $above_fold_selectors ) { - if ( ! isset( $lazyload_selectors[ $selector ] ) ) { - continue; - } - if ( empty( $above_fold_selectors ) ) { - $css_selectors[ $device ][] = 'html ' . strip_tags( $selector ) . ':not(.optml-bg-lazyloaded)'; - } else { - foreach ( $above_fold_selectors as $above_fold_selector => $bg_urls ) { - $css_selectors[ $device ][] = 'html ' . strip_tags( $selector ) . ':not(' . strip_tags( $above_fold_selector ) . '):not(.optml-bg-lazyloaded)'; - } - } - } + + $selectors = self::collect_device_selectors( $personalized_selectors, $lcp_data, $lazyload_selectors ); + // null means a client selector was unsafe, so the whole device rule is voided. + $tainted[ $device ] = null === $selectors; + $css_selectors[ $device ] = $tainted[ $device ] ? [] : $selectors; $preload_urls[ $device ] = []; - $css_selectors[ $device ] = array_unique( $css_selectors[ $device ] ); - if ( isset( $lcp_data['type'] ) && $lcp_data['type'] === 'bg' ) { - if ( ! empty( $lcp_data['bgSelector'] ) ) { - $css_selectors[ $device ] = array_map( - function ( $selector ) use ( $lcp_data ) { - return $selector . ':not(' . strip_tags( $lcp_data['bgSelector'] ) . ')'; - }, - $css_selectors[ $device ] - ); - } - if ( ! empty( $lcp_data['bgUrls'] ) ) { - $preload_urls[ $device ] = array_merge( $preload_urls[ $device ], $lcp_data['bgUrls'] ); - } + if ( isset( $lcp_data['type'] ) && $lcp_data['type'] === 'bg' && ! empty( $lcp_data['bgUrls'] ) ) { + $preload_urls[ $device ] = array_merge( $preload_urls[ $device ], $lcp_data['bgUrls'] ); } } if ( OPTML_DEBUG ) { @@ -80,8 +84,19 @@ function ( $selector ) use ( $lcp_data ) { } $hide_rule = ' { background-image: none !important; }'; - $mobile_selectors = implode( ',', $css_selectors[ Profile::DEVICE_TYPE_MOBILE ] ); - $desktop_selectors = implode( ',', $css_selectors[ Profile::DEVICE_TYPE_DESKTOP ] ); + $mobile = $css_selectors[ Profile::DEVICE_TYPE_MOBILE ]; + $desktop = $css_selectors[ Profile::DEVICE_TYPE_DESKTOP ]; + + // Keep the surviving rule scoped when the other device was voided. + if ( $tainted[ Profile::DEVICE_TYPE_MOBILE ] !== $tainted[ Profile::DEVICE_TYPE_DESKTOP ] ) { + if ( $tainted[ Profile::DEVICE_TYPE_MOBILE ] ) { + return empty( $desktop ) ? '' : '@media (min-width: 600px) { ' . implode( ',', $desktop ) . $hide_rule . ' }'; + } + return empty( $mobile ) ? '' : '@media (max-width: 600px) { ' . implode( ',', $mobile ) . $hide_rule . ' }'; + } + + $mobile_selectors = implode( ',', $mobile ); + $desktop_selectors = implode( ',', $desktop ); if ( $mobile_selectors === $desktop_selectors ) { return empty( $mobile_selectors ) ? '' : $mobile_selectors . $hide_rule; @@ -98,4 +113,47 @@ function ( $selector ) use ( $lcp_data ) { $media_query = '@media (max-width: 600px) { ' . $mobile_selectors . $hide_rule . ' } @media (min-width: 600px) { ' . $desktop_selectors . $hide_rule . ' }'; return $media_query; } + /** + * Build the background-hide selectors for one device. + * + * @param array> $personalized_selectors Stored bg data: watcher => [ above-fold selector => urls ]. + * @param array $lcp_data Stored LCP data for the device. + * @param array $lazyload_selectors Allowed watcher selectors as keys. + * + * @return array|null Selector strings, or null when a client selector is unsafe (the device rule is voided). + */ + private static function collect_device_selectors( array $personalized_selectors, array $lcp_data, array $lazyload_selectors ) { + $css_selectors = []; + foreach ( $personalized_selectors as $selector => $above_fold_selectors ) { + if ( ! isset( $lazyload_selectors[ $selector ] ) ) { + continue; + } + if ( empty( $above_fold_selectors ) ) { + $css_selectors[] = 'html ' . strip_tags( $selector ) . ':not(.optml-bg-lazyloaded)'; + continue; + } + foreach ( $above_fold_selectors as $above_fold_selector => $bg_urls ) { + if ( ! self::is_safe_selector( $above_fold_selector ) ) { + return null; + } + $css_selectors[] = 'html ' . strip_tags( $selector ) . ':not(' . $above_fold_selector . '):not(.optml-bg-lazyloaded)'; + } + } + + $css_selectors = array_unique( $css_selectors ); + + if ( isset( $lcp_data['type'] ) && $lcp_data['type'] === 'bg' && ! empty( $lcp_data['bgSelector'] ) ) { + if ( ! self::is_safe_selector( $lcp_data['bgSelector'] ) ) { + return null; + } + $css_selectors = array_map( + function ( $selector ) use ( $lcp_data ) { + return $selector . ':not(' . $lcp_data['bgSelector'] . ')'; + }, + $css_selectors + ); + } + + return array_values( $css_selectors ); + } } diff --git a/tests/media_rename/test-attachment-replace.php b/tests/media_rename/test-attachment-replace.php index e643e97f..b5007b14 100644 --- a/tests/media_rename/test-attachment-replace.php +++ b/tests/media_rename/test-attachment-replace.php @@ -268,6 +268,98 @@ public function chmod( $file, $mode = false, $recursive = false ) { }; } + /** + * Replacing an SVG must run the same sanitizer as a normal upload. + */ + public function test_replace_sanitizes_svg() { + $id = self::factory()->attachment->create_upload_object( OPTML_PATH . 'tests/assets/sample.svg' ); + $file_path = ( new Optml_Attachment_Model( $id ) )->get_source_file_path(); + + $tmp_file = self::FILESTASH . 'replace-scripted.svg'; + file_put_contents( $tmp_file, '' ); + + $replacer = new Optml_Attachment_Replace( + $id, + [ + 'name' => 'replace-scripted.svg', + 'type' => 'image/svg+xml', + 'tmp_name' => $tmp_file, + ] + ); + + $this->assertTrue( $replacer->replace(), 'Replacement operation failed.' ); + + $contents = file_get_contents( $file_path ); + $this->assertStringContainsString( 'assertStringNotContainsString( 'assertStringNotContainsString( 'onload', $contents ); + + wp_delete_post( $id, true ); + } + + /** + * An SVG the sanitizer cannot parse must not replace the original file. + */ + public function test_replace_rejects_unsanitizable_svg() { + $id = self::factory()->attachment->create_upload_object( OPTML_PATH . 'tests/assets/sample.svg' ); + $file_path = ( new Optml_Attachment_Model( $id ) )->get_source_file_path(); + $original = file_get_contents( $file_path ); + + $tmp_file = self::FILESTASH . 'replace-broken.svg'; + file_put_contents( $tmp_file, 'not 'replace-broken.svg', + 'type' => 'image/svg+xml', + 'tmp_name' => $tmp_file, + ] + ); + + $this->assertWPError( $replacer->replace() ); + $this->assertSame( $original, file_get_contents( $file_path ), 'Original SVG was overwritten.' ); + + wp_delete_post( $id, true ); + } + + /** + * A failed sanitizer write must abort the replace and leave the original SVG untouched. + */ + public function test_replace_aborts_when_sanitizer_write_fails() { + if ( 0 === posix_getuid() ) { + $this->markTestSkipped( 'Running as root bypasses the file permission that forces the write to fail.' ); + } + + $id = self::factory()->attachment->create_upload_object( OPTML_PATH . 'tests/assets/sample.svg' ); + $file_path = ( new Optml_Attachment_Model( $id ) )->get_source_file_path(); + $original = file_get_contents( $file_path ); + + // Read-only tmp file: sanitize_svg() can read it but its file_put_contents() fails. + $tmp_file = self::FILESTASH . 'replace-readonly.svg'; + file_put_contents( $tmp_file, '' ); + chmod( $tmp_file, 0444 ); + + $replacer = new Optml_Attachment_Replace( + $id, + [ + 'name' => 'replace-readonly.svg', + 'type' => 'image/svg+xml', + 'tmp_name' => $tmp_file, + ] + ); + + $result = $replacer->replace(); + + chmod( $tmp_file, 0644 ); + + $this->assertWPError( $result, 'A failed sanitizer write must abort the replace.' ); + $this->assertSame( $original, file_get_contents( $file_path ), 'Original SVG was overwritten despite a failed sanitize.' ); + $this->assertStringNotContainsString( 'update( + 'service_data', + [ + 'cdn_key' => 'test123', + 'cdn_secret' => '12345', + 'whitelist' => [ 'example.com' ], + ] + ); + $settings->update( 'lazyload', 'enabled' ); + $settings->update( 'lazyload_type', 'viewport' ); + $settings->update( 'bg_replacer', 'enabled' ); + Optml_Lazyload_Replacer::instance()->init(); + $this->reset_background_selectors(); + } + + /** + * Clean up the test environment after each test. + */ + public function tearDown(): void { + $this->reset_background_selectors(); + Profile::reset_current_profile(); + parent::tearDown(); + } + + /** + * Selectors produced by the frontend getUniqueSelector() are safe to embed. + * + * @dataProvider safe_selectors + * + * @param string $selector Selector to check. + */ + public function test_is_safe_selector_accepts_generated_selectors( $selector ): void { + $this->assertTrue( Lazyload::is_safe_selector( $selector ) ); + } + + /** + * Plain selectors getUniqueSelector() emits. + * + * @return array + */ + public function safe_selectors(): array { + return [ + 'body' => [ 'body' ], + 'id' => [ '#hero' ], + 'body child' => [ 'body > div.wp-block-cover' ], + 'nested nth' => [ '#main > section.hero.is-dark:nth-of-type(2) > div.inner_wrap' ], + 'non ascii class' => [ 'body > div.café' ], + ]; + } + + /** + * Selectors carrying CSS metacharacters are treated as unsafe. + * + * @dataProvider unsafe_selectors + * + * @param mixed $selector Selector to check. + */ + public function test_is_safe_selector_rejects_metacharacters( $selector ): void { + $this->assertFalse( Lazyload::is_safe_selector( $selector ) ); + } + + /** + * Selectors that must not be embedded (injection or CSS-invalid, including Tailwind's `:` and `/`). + * + * @return array + */ + public function unsafe_selectors(): array { + return [ + 'issue payload' => [ self::PAYLOAD ], + 'tailwind colon' => [ 'body > div.md:flex' ], + 'tailwind slash' => [ 'body > div.w-1/2' ], + 'closing paren' => [ 'div)' ], + 'selector list' => [ 'div, body' ], + 'declaration' => [ 'div;color:red' ], + 'braces' => [ 'div{}' ], + 'attribute' => [ 'div[style]' ], + 'quote' => [ 'a"b' ], + 'newline' => [ "a\nb" ], + 'empty' => [ '' ], + 'array' => [ [ 'div' ] ], + ]; + } + + /** + * An injected above-fold selector voids the device rule instead of leaking CSS. + */ + public function test_personalized_css_drops_injected_selectors(): void { + $device_data = [ + 'bg' => [ + self::WATCHER => [ + self::PAYLOAD => [], + ], + ], + ]; + + $css = Lazyload::get_personalized_css( + [ + Profile::DEVICE_TYPE_MOBILE => $device_data, + Profile::DEVICE_TYPE_DESKTOP => $device_data, + ] + ); + + $this->assertStringNotContainsString( 'background:red', $css ); + $this->assertStringNotContainsString( self::PAYLOAD, $css ); + $this->assertSame( '', $css ); + } + + /** + * An injected LCP selector voids the device rule. + */ + public function test_personalized_css_drops_injected_lcp_selector(): void { + $device_data = [ + 'bg' => [ self::WATCHER => [ '#hero' => [] ] ], + 'lcp' => [ + 'type' => 'bg', + 'bgSelector' => self::PAYLOAD, + ], + ]; + + $css = Lazyload::get_personalized_css( + [ + Profile::DEVICE_TYPE_MOBILE => $device_data, + Profile::DEVICE_TYPE_DESKTOP => $device_data, + ] + ); + + $this->assertStringNotContainsString( 'background:red', $css ); + $this->assertSame( '', $css ); + } + + /** + * A safe selector followed by an unsafe one still voids the whole device rule. + */ + public function test_personalized_css_drops_rule_when_any_selector_unsafe(): void { + $device_data = [ + 'bg' => [ + self::WATCHER => [ + '#hero' => [], + self::PAYLOAD => [], + ], + ], + ]; + + $css = Lazyload::get_personalized_css( + [ + Profile::DEVICE_TYPE_MOBILE => $device_data, + Profile::DEVICE_TYPE_DESKTOP => $device_data, + ] + ); + + $this->assertStringNotContainsString( 'background:red', $css ); + $this->assertSame( '', $css ); + } + + /** + * When only one device is voided, the surviving device stays scoped to its own media query + * instead of leaking a media-query-less rule onto the voided device's viewport. + */ + public function test_personalized_css_scopes_surviving_device_to_media_query(): void { + $css = Lazyload::get_personalized_css( + [ + Profile::DEVICE_TYPE_MOBILE => [ 'bg' => [ self::WATCHER => [ self::PAYLOAD => [] ] ] ], + Profile::DEVICE_TYPE_DESKTOP => [ + 'bg' => [ self::WATCHER => [ '#hero' => [] ] ], + 'lcp' => [ + 'type' => 'bg', + 'bgSelector' => 'body > div.hero:nth-of-type(1)', + ], + ], + ] + ); + + $this->assertStringNotContainsString( 'background:red', $css ); + $this->assertStringNotContainsString( self::PAYLOAD, $css ); + // The surviving desktop rule must stay desktop-scoped and not apply on phones. + $this->assertStringStartsWith( '@media (min-width: 600px) {', $css ); + $this->assertStringNotContainsString( '@media (max-width: 600px)', $css ); + // The safe above-fold and LCP selectors are still rendered (the gating must not over-reject them). + $this->assertStringContainsString( ':not(#hero):not(.optml-bg-lazyloaded)', $css ); + $this->assertStringContainsString( ':not(body > div.hero:nth-of-type(1))', $css ); + } + + /** + * The mirror case: when desktop is voided, the surviving mobile rule stays mobile-scoped. + */ + public function test_personalized_css_scopes_surviving_mobile_device_to_media_query(): void { + $css = Lazyload::get_personalized_css( + [ + Profile::DEVICE_TYPE_MOBILE => [ 'bg' => [ self::WATCHER => [ '#hero' => [] ] ] ], + Profile::DEVICE_TYPE_DESKTOP => [ 'bg' => [ self::WATCHER => [ self::PAYLOAD => [] ] ] ], + ] + ); + + $this->assertStringNotContainsString( 'background:red', $css ); + $this->assertStringNotContainsString( self::PAYLOAD, $css ); + // The surviving mobile rule must stay mobile-scoped and not apply on desktop. + $this->assertStringStartsWith( '@media (max-width: 600px) {', $css ); + $this->assertStringNotContainsString( 'min-width', $css ); + $this->assertStringContainsString( ':not(#hero):not(.optml-bg-lazyloaded)', $css ); + } + + /** + * Reset the cached background lazyload selectors. + */ + private function reset_background_selectors(): void { + $property = new ReflectionProperty( Optml_Lazyload_Replacer::class, 'background_lazyload_selectors' ); + $property->setAccessible( true ); + $property->setValue( null, null ); + } +} diff --git a/tests/test-dashboard-widget.php b/tests/test-dashboard-widget.php new file mode 100644 index 00000000..1f86d260 --- /dev/null +++ b/tests/test-dashboard-widget.php @@ -0,0 +1,101 @@ +update( + 'service_data', + [ + 'cdn_key' => 'test-cdn-key', + 'cdn_secret' => 'test-cdn-secret', + 'api_key' => 'test-api-key', + 'visitors' => 20, + 'visitors_pretty' => '20', + 'visitors_limit' => 5000, + 'visitors_limit_pretty' => '5k', + 'compression_percentage' => 42.5, + 'traffic' => 12.3, + ] + ); + set_current_screen( 'dashboard' ); + } + + /** + * Clean up the test environment after each test. + */ + public function tearDown(): void { + global $wp_meta_boxes; + unset( $wp_meta_boxes['dashboard'] ); + wp_deregister_script( self::HANDLE ); + set_current_screen( 'front' ); + wp_set_current_user( 0 ); + parent::tearDown(); + } + + /** + * Subscribers get neither the widget nor its localized account data. + */ + public function test_widget_hidden_from_subscriber() { + global $pagenow, $wp_meta_boxes; + wp_set_current_user( self::factory()->user->create( [ 'role' => 'subscriber' ] ) ); + + $widget = new Optml_Dashboard_Widget(); + $widget->add_dashboard_widget(); + + $previous_pagenow = $pagenow; + $pagenow = 'index.php'; + $widget->enqueue_widget(); + $pagenow = $previous_pagenow; + + $this->assertFalse( isset( $wp_meta_boxes['dashboard']['normal']['core'][ self::HANDLE ] ) ); + $this->assertFalse( wp_script_is( self::HANDLE, 'registered' ) ); + } + + /** + * Administrators still get the widget. + */ + public function test_widget_shown_to_administrator() { + global $wp_meta_boxes; + wp_set_current_user( self::factory()->user->create( [ 'role' => 'administrator' ] ) ); + + ( new Optml_Dashboard_Widget() )->add_dashboard_widget(); + + $this->assertTrue( isset( $wp_meta_boxes['dashboard']['normal']['core'][ self::HANDLE ] ) ); + } + + /** + * The localized service data only holds the stats the widget displays. + */ + public function test_localized_service_data_excludes_credentials() { + $method = new ReflectionMethod( Optml_Dashboard_Widget::class, 'get_script_localization' ); + $method->setAccessible( true ); + + $service_data = $method->invoke( new Optml_Dashboard_Widget() )['serviceData']; + + $this->assertEqualsCanonicalizing( + [ 'visitors', 'visitors_pretty', 'visitors_limit', 'visitors_limit_pretty', 'compression_percentage', 'traffic' ], + array_keys( $service_data ) + ); + $this->assertSame( 20, $service_data['visitors'] ); + } +}