Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 5 additions & 4 deletions inc/admin.php
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -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;
}

/**
Expand Down
24 changes: 21 additions & 3 deletions inc/dashboard_widget.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

Expand All @@ -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;
}

Expand Down Expand Up @@ -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' ) ),
Expand All @@ -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<string, mixed>
*/
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.
*
Expand Down
6 changes: 6 additions & 0 deletions inc/media_rename/attachment_replace.php
Original file line number Diff line number Diff line change
Expand Up @@ -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' ) );
}
Comment thread
girishpanchal30 marked this conversation as resolved.
}

global $wp_filesystem;

if ( ! $wp_filesystem->move( $this->file['tmp_name'], $original_file, true ) ) {
Expand Down
114 changes: 86 additions & 28 deletions inc/v2/BgOptimizer/Lazyload.php
Original file line number Diff line number Diff line change
Expand Up @@ -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.
*
Expand All @@ -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 ) {
Expand All @@ -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;
Expand All @@ -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<string, array<string, mixed>> $personalized_selectors Stored bg data: watcher => [ above-fold selector => urls ].
* @param array<string, mixed> $lcp_data Stored LCP data for the device.
* @param array<string, bool> $lazyload_selectors Allowed watcher selectors as keys.
*
* @return array<int, string>|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 );
}
}
92 changes: 92 additions & 0 deletions tests/media_rename/test-attachment-replace.php
Original file line number Diff line number Diff line change
Expand Up @@ -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, '<svg xmlns="http://www.w3.org/2000/svg" onload="alert(1)"><script>alert(document.domain)</script><rect width="10" height="10"/></svg>' );

$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( '<rect', $contents );
$this->assertStringNotContainsString( '<script', $contents );
$this->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 <svg an <<< xml document' );

$replacer = new Optml_Attachment_Replace(
$id,
[
'name' => '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, '<svg xmlns="http://www.w3.org/2000/svg"><script>alert(1)</script></svg>' );
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( '<script', file_get_contents( $file_path ) );

wp_delete_post( $id, true );
}

private function do_replace_test( $id_to_replace, $replace_file, $source_scaled, $result_scaled ) {
// Removed var_dump

Expand Down
Loading
Loading