721830 - WP Extensions for SVG support

Status: RESOLVED FIXED

(mozilla.org :: Security Assurance: Applications, task)

Description

[Jake Maul [:jakem]]

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?

Comment 1

[Curtis Koenig [:curtisk-use curtis.koenig+bzATgmail.com]]]

assigning to mgoodwin for review

Comment 2

[David Chan [:dchan]]

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

Comment 3

[David Chan [:dchan]]

jakem: Is there an update on the status of this plugin? Please see above concerns.

Comment 4

[Jake Maul [:jakem]]

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.

Comment 5

[Jake Maul [:jakem]]

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.

Comment 6

[Jake Maul [:jakem]]

Found direct contact information, email sent.

Comment 7

[Sterling Hamilton]

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:
[svg src="/wp-content/uploads/1900/1/example.svg" width="300" height="300" style="display:block; margin:auto;" type="embed"]

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

[David Chan [:dchan]]

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

[Sterling Hamilton]

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

Thanks for brining it up gents.

Comment 10

[Sterling Hamilton]

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.

Comment 11

[Jake Maul [:jakem]]

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

Comment 12

[David Chan [:dchan]]

(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

[David Chan [:dchan]]

(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

Comment 14

[Jake Maul [:jakem]]

Awesome, thanks! I'll re-open the blocked bug and get this deployed. This bug is done. :)