16 Jan 2018

Making Sure That Valid Values Are Provided For Shortcode Attributes Can Prevent Security Issues As Well Providing a Better Experience

One of the areas of WordPress plugins that has received additional attention when it comes to security recently has been shortcodes, as WordPress now allows anyone that is logged in to WordPress to access those. While that change has expanded the pool of people that might exploit an issue related to those, it was already the case that lower level users could access those and proper security should have been place, which hasn’t always been the case. Making sure things are done securely doesn’t just protect against vulnerabilities, but can provide a better experience for users, as can be seen with the plugin Power Charts.

Recently we were contacted by one of the users of our service, J.D. Grimes, who had found some possible vulnerabilities that involved shortcodes and another issue that we will get to in a moment. He was too busy to go further with them at the time and was wondering if we could take it from there in confirming them and getting in touch with the developers. One of the impacted plugins was Power Charts.

That plugin registers a couple of shortcodes that then call the function pc_shortcode():

28
29
add_shortcode( 'pc', array( &$this, 'pc_shortcode' ) );
add_shortcode( 'power-charts', array( &$this, 'pc_shortcode' ) );

Here is how that function looks as of version 0.1.0:

public function pc_shortcode( $atts ) {

	/* Get power charts attributes from the shortcode. */
	extract( shortcode_atts( array(
		'id'    => '',
		/*'group' => '',
		'num'   => '-1',
		'rnd'   => false,
		'no_excerpt' => '0',
		'no_company' => '0',
		'no_name' => '0',
		'no_image' => '0',
		'no_link' => '0',
		'render' => '',
		'template' => '',*/
	), $atts ) );

	$data = get_post_meta( $id, '_wpgo_power_charts_cpt_data', true );
	$chart_fixed_js = get_post_meta( $id, '_wpgo_power_charts_cpt_js', true );
	$chart_config_js = get_post_meta( $id, '_wpgo_power_charts_cpt_config_js', true );

	$css = get_post_meta( $id, '_wpgo_power_charts_cpt_css', true );
	$html = '<div class="pc-' . $id . ' wpgo-power-charts"></div>';
	$chart_js = "(function (){" . $chart_config_js . $chart_fixed_js;

	//echo "<pre>";
	//echo $chart_js;
	//echo "</pre>";

	// Only add chart scripts/styles to pages shortcode is used on
	wp_enqueue_script( 'wpgo-d3', $this->module_roots['uri'] . '/js/pcd3.js' , array(), '', true );
	wp_localize_script( 'wpgo-d3', 'pc_data_' . $id, $data );
	wp_add_inline_script( 'wpgo-d3', $chart_js );

	wp_enqueue_style( 'wpgo-power-charts', $this->module_roots['uri'] . '/css/power-charts.css' );
	wp_add_inline_style( 'wpgo-power-charts', $css );

	return html_entity_decode($html);
}

The variable $att in that contains attributes that are included with a short code. For example, if you wanted to show the chart with Chart ID 1 you would use this short code:

[power-charts id="1"]

The line that begins “extract” will set what is in the “id” to the variable $id in the function. That code doesn’t place any restriction on what can be user can cause $id to be set to or check if it is any way valid.

Where that becomes a security issue is that code will cause the value of the variable to be output in two locations, which can be used to cause persistent cross-site scripting to happen by including JavaScript code in the value.

The first line is more obvious, as the value is added to a variable $html:

$html = '<div class="pc-' . $id . ' wpgo-power-charts"></div>';

Which you can probably guess will be included the HTML code of the resulting page where the shortcode is used.

The second area is the one J.D. was looking into when he came across the issue with this plugin, which is in wp_localize_script():

wp_localize_script( 'wpgo-d3', 'pc_data_' . $id, $data );

The second parameter in that is directly output on the page, so the value needs to be properly secured, but that hasn’t happened here.

Since the value should only be an integer, limiting the value of $id to those would fix this. If the code then checked to make sure that a valid value was included and warned if it wasn’t, that could provide users with help if they have not correctly set up a shortcode.

We notified the developer of the issue on December 11th and the developer responded the same day that they were working on a fix, but the vulnerability has not been fixed so far.

Proof of Concept

The following shortcode will cause an alert box that says “XSS” to be shown on the front-end page when placed in a WordPress post or page:

[power-charts id='test= "1";alert("XSS"); var test2 ']

Timeline

  • December 11, 2017 – Developer notified.
  • December 11, 2017 – Developer responds.

Concerned About The Security of the Plugins You Use?

When you are a paying customer of our service, you can suggest/vote for the WordPress plugins you use to receive a security review from us. You can start using the service for free when you sign up now. We also offer security reviews of WordPress plugins as a separate service.

Leave a Reply

Your email address will not be published.