Anatomy of a Shortcode with its Flaws

During my research in free extensions, or during code audits ordered by customers, I find from time to time things so simple to correct and yet so devastating that I wanted to show you one, a beautiful one.

The Shortcode

A shortcode takes this simple form when writing an article since WordPress 2.5 (March 2008): [shortcode param=""] or [shortcode]content[/shortcode].

The shortcode has the advantage of being easy to use, of avoiding leaving access to PHP, or HTML, or JavaScript code to use a function with parameters.

Let’s take a simple example of a shortcode which is by default in WordPress, the shortcode video, demo :

[video src="https://www.youtube.com/watch?v=7gicRzTTC9Q"]

With this method, only videos from recognized providers can be loaded, or local videos.

Another example with [gallery] which displays media from our site:

[gallery ids="5358825,5119297,5119245,5119151"], demo :

Here a little more complicated in the core, but for a user it is easy to give the media IDs so that WordPress will add these media type articles to display them.

Without shortcode, you would then have to create a PHP function that does a loop, displays images, etc., so you need PHP and allow the text editor to also become a code editor, and that’s no.

Leaving the possibility of creating PHP code from the code editor is simply unthinkable!

Create a Shortcode

You can also very easily create a shortcode, it’s really simple, here are 2 examples (spoiler alert, I put vulnerabilities in them, I say it before for those who will copy/paste to use them):

Shortcode “post title” (with a free flaw)

<?php
add_shortcode( 'title', 'sc_post_title' );
function sc_post_title( $attr ) {
	$atts    = shortcode_atts( [ 'id' => 0, 'bold' => 0 ], $attr, 'date' );
	$post_id = $atts['id'];
	if ( ! $post_id ) {
		return "Error $post_id incorrect.";
	};
	$title = get_the_title( $post_id );
	if ( $atts['bold'] ) {
		$title = '<strong>' . str_replace( [ '<b>', '</b>', '<strong>', '</strong>' ], '', $title ) . '</strong>';
	}
	return $title;
}

Do you have the first flaw? No ? It’s not very bad (at first) but we can therefore display the title of any post, private, of any type, in trash, etc. This is then a simple “Information Disclosure”. Some extensions use the post_title to store other things, for example a newsletter plugin can very well create its subscribers in post_type “my_subscribers” and the post_title is their email address! I’ve already seen it so yes it exists… We’re going to talk about “Private Data Disclosure” now, the GDPR doesn’t like that!

What is missing here is verification that this type of article is intended to publicly displays its content. So here is the fix:

<?php
add_shortcode( 'title', 'sc_post_title' );
function sc_post_title( $attr ) {
	$atts    = shortcode_atts( [ 'id' => 0, 'bold' => 0 ], $attr, 'date' );
	$post_id = $atts['id'];
	if ( ! $post_id || ! is_post_publicly_viewable( $post_id ) ) {
		return "Error $post_id incorrect.";
	};
	$title = get_the_title( $post_id );
	if ( $atts['bold'] ) {
		$title = '<strong>' . str_replace( [ '<b>', '</b>', '<strong>', '</strong>' ], '', $title ) . '</strong>';
	}
	return $title;
}

We now check that the post in question is public and visible, therefore its type and status with the dedicated WordPress function is_post_publicly_viewable().

Another flaw? Yes, there are 2 others, XSS vulnerabilities. Just type this in the article editor:

[[title id="Hello there"]] and the screen displays “Error Hello there incorrect.” We still can’t put “script” tags because it remains the editor which will be filtered because I am a Contributor, but you still have to display this kind of content in an esc_html()! Any data coming from a user must be treated as malicious, basics.

The second one is wanting to remove the bold tags to frame the title in bold. The problem is that we can now display script tags… Simply point to the article you are writing and put in the title “<sc<b>ript>alert(123) ;</sc<b>ript>“, now I can save my draft, there are no valid script tags to remove, and <b> tags are allowed. But when passing through shortcode, the <b> tags are removed, leaving room for the concatenation of the rest which will finally give “<script>alert(123);</script>“. XSS.

Shortcode “post title 2” (with a free flaw)

What if we needed to display the title of an element of a custom table, we cannot use “get_post()” or anything like that, we must create our own SQL query. Let’s create this new shortcode.

add_shortcode( 'title', 'sc_post_title' );
function sc_post_title( $attr ) {
	global $wpdb;
	$atts    = shortcode_atts( [ 'id' => 0 ], $attr, 'date' );
	$post_id = $atts['id'];
	if ( ! $post_id || ! intval( $post_id ) ) {
		return "Error ID incorrect.";
	};
	$sql = "SELECT * FROM {$wpdb->posts} WHERE ID = $post_id LIMIT 1";
	$req = $wpdb->get_row( $sql );
	if ( empty( $req ) ) {
		return "No results.";
	}
	return esc_html( $req->post_title );
}

Haaa finally security! We don’t display the ID, we use intval() to make sure it’s a number, we even do esc_html()!

And yet… there is indeed a bonus flaw and not the least because here we are going to talk about a SQL Injection flaw. Let’s see how we can successfully exploit this.

But first if you don’t know what we’re talking about, please read the OWASP website. Basically, it’s about successfully manipulating a database query in order to display/add/modify/delete data theoretically not accessible in the original scenario.

We assume that I am a simple Contributor on this site, who does not have access to the source code of the shortcode and therefore cannot know the potential flaws. I’ll have to test and try, this is how a Web Security Consultant (or Security Researcher or Ethical Hacker too) does it.

Here is already the basis, its normal use:

Which gives in front:

Good, we have the correct title, post 1 from my custom table.

How to know if there is a SQLi? Let’s try some values!

Testing with "-1" then "-1 or 1=1", if I still have the correct title with my 2nd attempt, I would have injected SQL code (the OR) into the query which should only take the numbers, right?

It works!

But then, just how did it get past testing and verification?

Read the shortcode again, there is none. The intval() only tests that what is entered is indeed an integer if it is cast as a number and this is the case of 1 or 1=1 which gives “1” just like 123hello gives “123 “. But it’s just a condition here, the value of $post_id was not assigned, that’s the error.

So can I go read another table or not? And any field or not? Yes and no, “it depends”.

If you want to read another table, you have to know its name. And you’re going to tell me “but we know the name of the tables since we know WordPress 😀” yes but what about the prefix? If it’s wp_ like it is by default, ok, but what if it was changed?

(Reassure me, you changed it right? Otherwise do it before reading the rest, I am not responsible for the next heart attack that you will have by continuing to read the article with a wp_ prefix… SecuPress Pro can do it for you!)

How can we successfully read a second table in a query – the content of which we have no knowledge of – in the same query? With the keyword UNION! However, it is of course necessary that the 2 queries have the same number of fields in their output, otherwise, error… Let’s start by finding how many fields are in the original table by adding with the SELECT in our vulnerable shortcode:

I run a SELECT in no table but I choose my own value, yep we can do this. I did it as many times as necessary until the front result changed and showed me:

"f"! So the table in which the query is made contains 6 columns if it is a “SELECT *” or simply the query makes a SELECT of 6 fields, same result, so we know that I can display here what I want in the 6th field.

So we try to read the wp_users table and the user_email of account ID=1?

Here is the query injected into my shortcode and here is the front end result:

Too bad! The table prefix is not wp_! This is where the majority of automated hacks stop.

But… if your SQL user is not properly configured and the hacker is targeting you, then he may be able to read the prefix differently!

If our user has the rights, he can read the INFORMATION_SCHEMA table! Which gives us :

Boom, here is our prefix of the first table found, 99% chance (arbitrary data) that it is with the prefix of this WP, at worst we shift the query by a table until. Here we will be able to start reading our user_email again.

Which leads to

“contact@secupress.me” is the admin e-mail ID 1!

Ok, this email is not necessarily secret, it does not represent a big step forward, but I’ll let you imagine what a hacker can read in the database in the options tables (tokens? licenses?), in meta posts (sales data? emails?), etc etc

Shortcode fix

The correction is again very simple, please only think about this: prepare.

I still add a real integer cast for our post_id but if it was text we would have to prepare it anyway!

<?php
add_shortcode( 'title', 'sc_post_title' );
function sc_post_title( $attr ) {
	global $wpdb;
	$atts    = shortcode_atts( [ 'id' => 0 ], $attr, 'date' );
	$post_id = (int) $atts['id'];
	if ( ! $post_id ) {
		return "Error ID incorrect.";
	};
	$sql = "SELECT * FROM {$wpdb->prefix}custom_table WHERE ID = %d LIMIT 1";
	$req = $wpdb->get_row( $wpdb->prepare( $sql, $post_id ) );
	if ( empty( $req ) ) {
		return "No results.";
	}
	return esc_html( $req->custom_title );
}

Read the complete documentation on WordPress for $wpdb and read also the SQL one!

Everyone is counting on you.

About Julio Potier All of Julio Potier's posts

Creator of SecuPress and the LearnWPSecurity Training, Julio is also lead organisator of WordCamp Lille.
Compulsive speaker, Senior Trainer and WordPress Engineer, he's a specialist in security since 2002 and contribute to WordPress various ways.