WP Extensions for SVG support

RESOLVED FIXED

Status

mozilla.org
Security Assurance: Applications
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: jakem, Unassigned)

Tracking

Details

(Whiteboard: [secr:dchan], URL)

(Reporter)

Description

5 years ago
Wordpress does not natively support SVG files... the recommended solution is to install an extension that adds support. This seems to be the most popular one (I guess not many people care about SVG in WP), and looks to be pretty simple.

http://wordpress.org/extend/plugins/scalable-vector-graphics-svg/

Ack?
Keywords: sec-review-needed
Whiteboard: [pending secreview]
assigning to mgoodwin for review
Whiteboard: [pending secreview] → [secr:mgoodwin]

Comment 2

5 years ago
Sorry Mark, going to take this one from you.

There appears to be XSS in the plugin. The handler code doesn't escape content supplied by the user.

When content is enclosed, the complete shortcode macro including its content will be replaced with the function output. It is the responsibility of the handler function to provide any necessary escaping or encoding of the raw content string, and include it in the output. [1]

XSS should be possible by placing specifying an invalid attribute that has XSS or by setting a value with XSS with appropriate escapes "

We would need to modify the library to encode the values and to escape the attributes.

Vulnerable lines of code
 40   function process_shortcode( $atts ) {
 41     $valid_attributes = array( 'src' , 'style' , 'type' , 'width' , 'height'     );
 42 
 43     $content = NULL;
 44 
 45     foreach( $atts as $attribute => $value ) {
 46       if( ! in_array( $attribute , $valid_attributes ) ) {
 47         $content .= "\n" . '<!-- Invalid attribute ignored: ' . $attribute .     ' -->' . "\n";
 48       }
 49     }
 50 
 51     switch( $atts[ 'type' ] ) {
 52       case 'iframe':
 53         $content .= '<iframe src="' . $atts[ 'src' ] . '" width="' . $atts[     'width' ] . '" height="' . $atts[ 'height' ] . '" style="' . $atts[ 'style'     ] . '">';
 54         $content .= '</iframe>';
 55       break;
 56       case 'embed':
 57       default:
 58         $content .= '<embed src="' . $atts[ 'src' ] . '" width="' . $atts[ '    width' ] . '" height="' . $atts[ 'height' ] . '" ';
 59         $content .= 'type="image/svg+xml" pluginspage="http://www.adobe.com/    svg/viewer/install/" style="' . $atts[ 'style' ] . '" /> ';
 60       break;
 61     }
 62 
 63     return $content;
 64   }
 65 
 66 }

[1] - http://codex.wordpress.org/Shortcode_API#Enclosing_vs_self-closing_shortcodes
Whiteboard: [secr:mgoodwin] → [secr:dchan]

Comment 3

5 years ago
jakem: Is there an update on the status of this plugin? Please see above concerns.
Keywords: sec-review-needed → sec-review-complete
(Reporter)

Comment 4

5 years ago
No, there is no update. I can attempt to notify the author of this, but have no direct contact information for them. I can't say when (or if) there would ever be a fix... it doesn't seem to be regularly maintained, but perhaps that's just because it "just works" and the maintainer has no additional plans for it.
QA Contact: mcoates → jstevensen
(Reporter)

Comment 5

5 years ago
I have posted to the relevant WP forum about this (not the vuln itself, but that there might be one), but no response. I haven't found actual author contact information to reach him directly.
(Reporter)

Comment 6

5 years ago
Found direct contact information, email sent.

Comment 7

5 years ago
Howdy! Developer here.

Could you provide an example of the exploit?

The UseCase of the plugin is that users who have access to the Administration menu and can add content use the shortcode.

Example:
&#91;svg src="/wp-content/uploads/1900/1/example.svg" width="300" height="300" style="display:block; margin:auto;" type="embed"&#93;

If we are assuming that the WP install allows non-registered users to write content (comments) OR that the site allows for open publishing - then I can see where this would be a problem.

Just figured I'd ask before I got started.

Comment 8

5 years ago
The risk is reduced if only administrative users can upload an svg file and specify the parameters. 

There is still the risk for a cross-site request forgery if an admin user is logged in and visits a malicious site. The site could then potentially perform/trick the admin into uploading a svg file with malformed attrs. However this would have to be a targeted attack on the admin/site.

I don't recall seeing a csrf-token in the code, but maybe WP offers that protection by default.

Comment 9

5 years ago
I'll take a look for the CSRF stuff and I'll lock it down.

Thanks for brining it up gents.

Comment 10

5 years ago
OK! So...IMO that plugin sucked before. I did it late at night for myself. Best code of my life naturally.

The new one (2.1.1) as of this post, doesn't require short codes. Doesn't require short codes, no need to validate, just adds the proper mime/type. You can use them as normal images now.
(Reporter)

Comment 11

5 years ago
@dchan: can we get a re-review of this plugin?

Comment 12

5 years ago
(In reply to Jake Maul [:jakem] from comment #11)
> @dchan: can we get a re-review of this plugin?

I'll add it to my tasks. How soon is this needed?

Comment 13

5 years ago
(In reply to Sterling Hamilton from comment #10)
> OK! So...IMO that plugin sucked before. I did it late at night for myself.
> Best code of my life naturally.
> 
> The new one (2.1.1) as of this post, doesn't require short codes. Doesn't
> require short codes, no need to validate, just adds the proper mime/type.
> You can use them as normal images now.

Nice!

Can't see anything wrong with the new code :)

@jakem: Well the review is done now
(Reporter)

Comment 14

5 years ago
Awesome, thanks! I'll re-open the blocked bug and get this deployed. This bug is done. :)
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Keywords: sec-review-complete
You need to log in before you can comment on or make changes to this bug.