Skip to content
Closed
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
18 changes: 18 additions & 0 deletions safe-svg.php
Original file line number Diff line number Diff line change
Expand Up @@ -149,6 +149,8 @@ function ( $tabs ) {

// Init all the things.
add_action( 'init', array( $this, 'setup_blocks' ) );
add_action( 'init', array( $this, 'register_svg_mime_type' ) );
add_filter( 'wp_check_filetype_and_ext', array( $this, 'fix_mime_type_svg' ), 100, 4 );
Comment on lines +152 to +153
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The concern with adding these hooks is that once SVGs are globally allowed via upload_mimes, WordPress will accept SVG uploads regardless of whether they pass through Safe SVG’s sanitizer.

In core, SVG uploads are typically validated because most upload paths go through wp_handle_upload() or wp_handle_sideload(), which we explicitly hook into via wp_handle_upload_prefilter and wp_handle_sideload_prefilter.

However, both of those functions are thin wrappers around _wp_handle_upload(), and that lower-level function can be called directly with any $action value (see core code).

When _wp_handle_upload() is called with a custom action (for example 'wp_upload_test'), the resulting prefilter becomes:

wp_upload_test_prefilter

Safe SVG does not hook into arbitrary variations of this filter. We intentionally only hook into the core upload and sideload prefilters.

If SVGs are globally permitted at the MIME level, and a plugin or theme calls _wp_handle_upload() with a non-standard action, that SVG upload will be accepted without ever running through the sanitizer. This creates a potential security vulnerability.

The key issue here is that we cannot assume third-party plugins will follow WordPress core’s upload flow. Nothing enforces that, and many plugins already call _wp_handle_upload() directly. Allowing SVGs globally means we implicitly trust all of those callers, which Safe SVG should not do.

Historically, this is why these hooks were removed in #228.

Regarding WP Offload Media: if it needs SVG MIME support, a safer approach would be one of the following:

  • Provide a temporary or scoped filter that allows SVG MIME types only during a controlled upload flow
  • Require integrations to explicitly opt in or add their own MIME handling, rather than enabling SVGs globally

Globally enabling SVG MIME types without guaranteeing sanitizer coverage across all upload paths is not something we can safely do.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good points. Thanks for the thorough review. The readme updates at least give everyone a heads up so they can adjust appropriately.

add_filter( 'wp_handle_sideload_prefilter', array( $this, 'check_for_svg' ) );
add_filter( 'wp_handle_upload_prefilter', array( $this, 'check_for_svg' ) );
add_filter( 'wp_prepare_attachment_for_js', array( $this, 'fix_admin_preview' ), 10, 3 );
Expand All @@ -173,6 +175,22 @@ public function allow_svg_from_upload() {
add_filter( 'wp_check_filetype_and_ext', array( $this, 'fix_mime_type_svg' ), 75, 4 );
}

/**
* Register SVG MIME type globally for proper detection by third-party plugins.
*
* This ensures wp_check_filetype() can identify SVG files in all contexts,
* not just during specific admin page loads. This fixes compatibility with
* plugins like WP Offload Media that process files outside of the standard
* WordPress upload flow.
*
* @since 2.4.1
*
* @return void
*/
public function register_svg_mime_type() {
add_filter( 'upload_mimes', array( $this, 'allow_svg' ) );
}

/**
* Custom function to check if user can upload svg.
*
Expand Down
Loading