Last Comment Bug 721830 - WP Extensions for SVG support
: WP Extensions for SVG support
Status: RESOLVED FIXED
[secr:dchan]
:
Product: mozilla.org
Classification: Other
Component: Security Assurance: Applications (show other bugs)
: other
: x86 Mac OS X
: -- normal (vote)
: ---
Assigned To: security-assurance
: Joe Stevensen [:joe]
Mentors:
http://wordpress.org/extend/plugins/s...
Depends on:
Blocks: 720862
  Show dependency treegraph
 
Reported: 2012-01-27 11:57 PST by Jake Maul [:jakem]
Modified: 2012-08-15 06:17 PDT (History)
4 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---


Attachments

Description Jake Maul [:jakem] 2012-01-27 11:57:11 PST
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]] 2012-02-22 14:54:29 PST
assigning to mgoodwin for review
Comment 2 David Chan [:dchan] 2012-02-22 15:04:48 PST
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] 2012-03-06 09:48:50 PST
jakem: Is there an update on the status of this plugin? Please see above concerns.
Comment 4 Jake Maul [:jakem] 2012-03-06 10:13:19 PST
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] 2012-03-19 16:09:15 PDT
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] 2012-03-22 16:25:14 PDT
Found direct contact information, email sent.
Comment 7 Sterling Hamilton 2012-03-22 17:08:20 PDT
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 David Chan [:dchan] 2012-03-28 15:26:47 PDT
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 2012-03-28 15:30:20 PDT
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 2012-04-12 20:02:38 PDT
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] 2012-04-16 14:03:37 PDT
@dchan: can we get a re-review of this plugin?
Comment 12 David Chan [:dchan] 2012-04-18 11:12:03 PDT
(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] 2012-04-18 11:15:51 PDT
(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] 2012-04-18 11:59:02 PDT
Awesome, thanks! I'll re-open the blocked bug and get this deployed. This bug is done. :)

Note You need to log in before you can comment on or make changes to this bug.