Skip to content

Icons: Skip sanitizing the icons bundled with WordPress #14492

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
fushar wants to merge 2 commits into
base: trunk
Choose a base branch
from
Open

Icons: Skip sanitizing the icons bundled with WordPress #14492

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
887e766
Icons: Skip sanitizing the icons bundled with WordPress.
fushar Oct 19, 2026
fa2f8de
Address comments
fushar Oct 23, 2026
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
32 changes: 31 additions & 1 deletion src/wp-includes/class-wp-icons-registry.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,14 @@ class WP_Icons_Registry {
*/
protected $registered_icons = array();

/**
* Resolved path of the core icon library directory.
*
* @since 7.2.0
* @var string|false|null
*/
private $core_icon_library_directory = null;

/**
* Container for the main instance of the class.
*
Expand Down Expand Up @@ -357,6 +365,7 @@ protected function sanitize_icon_content( $icon_content ) {
* Retrieves the content of a registered icon.
*
* @since 7.0.0
* @since 7.2.0 Core icons skip sanitization.
*
* @param string $icon_name Icon name including namespace.
* @return string|null The content of the icon, if found.
Expand All @@ -380,7 +389,12 @@ protected function get_content( $icon_name ) {
return null;
}

$content = $this->sanitize_icon_content( file_get_contents( $icon_path ) );
$content = file_get_contents( $icon_path );

// Core icons are trusted, so skip the costly sanitization.
if ( ! $this->is_core_icon_file( $icon_path ) ) {
$content = $this->sanitize_icon_content( $content );
}

if ( empty( $content ) ) {
wp_trigger_error(
Expand All @@ -395,6 +409,22 @@ protected function get_content( $icon_name ) {
return $this->registered_icons[ $icon_name ]['content'];
}

/**
* Determines whether an icon file is a core icon.
*
* @since 7.2.0
*
* @param string $icon_path Icon file path, resolved with `realpath()`.
* @return bool Whether the file is in the core icon library.
*/
private function is_core_icon_file( $icon_path ) {
if ( null === $this->core_icon_library_directory ) {
$this->core_icon_library_directory = realpath( ABSPATH . WPINC . '/images/icon-library' );
}

return is_string( $this->core_icon_library_directory ) && str_starts_with( $icon_path, $this->core_icon_library_directory . DIRECTORY_SEPARATOR );
}

/**
* Retrieves an array containing the properties of a registered icon.
*
Expand Down
4 changes: 2 additions & 2 deletions tests/phpunit/tests/icons/wpGetIcon.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,11 +47,11 @@ public function test_wp_get_icon_returns_empty_string_for_unknown_icon() {

/**
* @ticket 64847
* @ticket 66338
*/
public function test_wp_get_icon_default_attributes() {
$output = wp_get_icon( 'core/plus' );
// WP_HTML_Tag_Processor lowercases attribute names.
$this->assertStringContainsString( 'viewbox="0 0 24 24"', $output );
$this->assertStringContainsStringIgnoringCase( 'viewbox="0 0 24 24"', $output );
$this->assertStringContainsString( 'width="24"', $output );
$this->assertStringContainsString( 'height="24"', $output );
$this->assertStringContainsString( 'aria-hidden="true"', $output );
Expand Down
33 changes: 33 additions & 0 deletions tests/phpunit/tests/icons/wpIconsRegistry.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -499,6 +499,39 @@ public function test_get_content_reads_from_valid_file_path() {
$this->assertStringContainsString( '<path', $icon['content'] );
}

/**
* @ticket 66338
*
* @covers ::get_content
*/
public function test_get_content_sanitizes_non_core_icon_file() {
$path = $this->create_temp_icon_file( '<svg><script>alert( 1 )</script><path d="M0 0"/></svg>' );
Comment thread
fushar marked this conversation as resolved.

$this->registry->register(
'test-collection/from-file',
array(
'label' => 'From File',
'file_path' => $path,
)
);

$icon = $this->registry->get_registered_icon( 'test-collection/from-file' );
$this->assertStringContainsString( '<path', $icon['content'] );
$this->assertStringNotContainsString( '<script', $icon['content'] );
}

/**
* Should serve a core icon unsanitized (sanitizing would lowercase `viewBox`, which would fail this test).
*
* @ticket 66338
*
* @covers ::get_content
*/
public function test_get_content_does_not_sanitize_core_icon() {
$icon = $this->original_registry->get_registered_icon( 'core/pencil' );
$this->assertSame( file_get_contents( ABSPATH . WPINC . '/images/icon-library/pencil.svg' ), $icon['content'] );
}

/**
* Provides icon files that cannot yield valid content.
*
Expand Down
Loading