xss ?

We fixed this. The current version of the plugin has this fixed.

the version 2.1.12 doesn’t seems to escape page_links

I will reach out to our team to let them know, thanks.

any news ?

I will need to ping them again.

Hello folks,

How about you guys do something?
It’s been weeks and it still hasn’t been fixed. Given the popularity of the plugin, this needs an update asap.

I don’t want to go through the trouble of contacting the WP Team (but I’ll do if I have to).

Did you download 2.1.14?

Well, obviously I did.

You have 4 files impacted. Two of them use urlencode (further investigation is needed to see if it’s good enough or not), and two of them don’t seem to escape anything at all. Are those two vulnerable to outside attacks or not, that is for you to investigate. I personally believe they are.

We’re using a built in function from WP.

https://codex.www.ads-software.com/Function_Reference/paginate_links

WordPress uses it much in the same way:

https://github.com/WordPress/WordPress/blob/4.2.2/wp-admin/includes/media.php

May I ask why you think this is an issue?

Hi,

The problem here isn’t with the paginate_links directly, but with your usage of add_query_arg(), which is unsecure.

As you can see in their media.php file, WordPress escapes the result of add_query_arg() with the function esc_url(). The difference is, you don’t do that, and that’s where the core of the problem is.

Example of one of your files:

$page_links = paginate_links( array(
  'base' => add_query_arg( 'paged', '%#%' ),
  'format' => '',
  'prev_text' => __('?', 'jw-player-plugin-for-wordpress'),
  'next_text' => __('?', 'jw-player-plugin-for-wordpress'),
  'total' => $total,
  'current' => $paged,
  'add_args' => array('playlist' => $current_playlist, 'orderby' => $order_by, 'order' => $order)
));

In case your team missed the important announcement by WordPress, you should have a read at this article for an understanding of the problem.

Be aware that two of your 4 occurrences use the urlencode function, and I have no idea if it makes the output 100% safe, as it doesn’t follow the exact recommendations by WP.

We did read the article and then went to Github to see how WordPress was dealing with this. Below you see the code in the referenced WordPress file of the latest version 4.2.2.

<?php
$page_links = paginate_links( array(
	'base' => add_query_arg( 'paged', '%#%' ),
	'format' => '',
	'prev_text' => __('&laquo;'),
	'next_text' => __('&raquo;'),
	'total' => ceil($wp_query->found_posts / 10),
	'current' => $q['paged'],
));
if ( $page_links )
	echo "<div class='tablenav-pages'>$page_links</div>";
?>

Their usage of add_query_arg is exactly the same as ours and they do not escape the output. As a result we assume we’re doing the right thing here, but we’re always glad with help. Why do you think it is a problem in our code but not a problem in the WordPress code? Thanks for your help.

Links are actually escaped inside the pagina_links function itself, which is the reason why we do not need to escape the url once more. See in the code:

https://github.com/WordPress/WordPress/blob/4.2.2/wp-includes/general-template.php#L2703
https://github.com/WordPress/WordPress/blob/4.2.2/wp-includes/general-template.php#L2718
https://github.com/WordPress/WordPress/blob/4.2.2/wp-includes/general-template.php#L2734

Hi,

You’re right, I actually didn’t check inside the paginate function itself, and it turns out WP did properly escape the paginate_links function with esc_url calls.
That means JWPlaylistManager.php and jwp6-playlist-manager.php should both be safe.

As for JWEmbedderConfig.php and jwp6-class-player.php, which both use urlencode, I don’t really have a good visibility on this. This would encode characters like the brackets so theoretically it’s all good, but on the other hand, esc_url does a lot more background work. I guess it depends on the URLs you are processing with those two files (I didn’t check that far).

In any case, that was my bad for not looking down enough in the code. Sorry!

Hopefully, better safe than sorry.

It’s ok, thank you for being so thorough! ??