add_query_arg() XSS Vulnerability

by

I’ve created a personal plugin and now I’m worried about this vulnerability: https://blog.sucuri.net/2015/04/security-advisory-xss-vulnerability-affecting-multiple-wordpress-plugins.html

This is how I use add_query_arg(). Is this fine or should I protect it in some way?

general_plugin.php

function add_custom_query_var( $vars ){
  $vars[] = "custom_var";
  return $vars;
}
add_filter( 'query_vars', 'add_custom_query_var' );

plugin_part1.php

...
echo '<a href="https://www.example.com/page/?custom_var=".$db_table->db_column."">text</a>....'
...

plugin_part1-fixed.php

...
echo '<a href="https://www.example.com/page/'.add_query_arg( "custom_var", $db_table->db_column).'">text</a>....'
...

plugin_part2.php

$id = intval(get_query_var( 'custom_var' ));
$sql = $mydb->prepare(
        "
        SELECT * 
        FROM `db_table` 
        WHERE `db_table`.`db_column` = %d
        ", 
        array($is)
);

1 Answer
1

Unless I’m missing something that is staring me in the face, you aren’t using add_query_arg() or remove_query_arg(); since those are the only functions affected by this particular exploit you should be safe.

Your code does use the query_vars filter and get_query_var() but neither of those are effected by the exploit you’ve referenced.

Otherwise your code looks good, I do see a typo in the second to last line of “plugin_part2.php”, it should probably be array($id). Without seeing the rest in context I can’t say that you are safe with 100% certainty, but nothing you’ve posted is vulnerable to the exploit you’re asking about.

For anyone who stumbles on this post, the fix for this particular exploit goes like this, any instance of:

add_query_arg($param1, $param2, $old_query_or_uri);

or

remove_query_arg($key, $query);

should be replaced with

esc_url(add_query_arg($param1, $param2, $old_query_or_uri));

or

esc_url(remove_query_arg($key, $query));

respectively.

An excellent write up of this exploit is available in this article.

Leave a Comment