Title: xss ? Last modified: August 24, 2016 --- # xss ? * [madri2](https://wordpress.org/support/users/madri2/) * (@madri2) * [11 years, 3 months ago](https://wordpress.org/support/topic/xss-1/) * hello, After reading this : [https://make.wordpress.org/plugins/2015/04/20/fixing-add_query_arg-and-remove_query_arg-usage/](https://make.wordpress.org/plugins/2015/04/20/fixing-add_query_arg-and-remove_query_arg-usage/) * do the files ./plugins/jw-player-plugin-for-wordpress/jwp6/jwp6-playlist-manager. php and ./plugins/jw-player-plugin-for-wordpress/media/JWPlaylistManager.php contains an XSS ? * the code is $page_links = paginate_links( array( ‘base’ => add_query_arg( ‘paged’,‘%#%’),‘ format’ => ”, ‘prev_text’ => __(‘«’), ‘next_text’ => __(‘»’), ‘total’ => $total,‘ current’ => $paged, ‘add_args’ => array(‘playlist’ => $current_playlist, ‘orderby’ => $order_by, ‘order’ => $order) )); and
* page_links is not escaped * [https://wordpress.org/plugins/jw-player-plugin-for-wordpress/](https://wordpress.org/plugins/jw-player-plugin-for-wordpress/) Viewing 14 replies - 1 through 14 (of 14 total) * Plugin Author [JW Player](https://wordpress.org/support/users/longtail-video/) * (@longtail-video) * [11 years, 3 months ago](https://wordpress.org/support/topic/xss-1/#post-6043686) * We fixed this. The current version of the plugin has this fixed. * Thread Starter [madri2](https://wordpress.org/support/users/madri2/) * (@madri2) * [11 years, 3 months ago](https://wordpress.org/support/topic/xss-1/#post-6043693) * the version 2.1.12 doesn’t seems to escape page_links * Plugin Author [JW Player](https://wordpress.org/support/users/longtail-video/) * (@longtail-video) * [11 years, 3 months ago](https://wordpress.org/support/topic/xss-1/#post-6043701) * I will reach out to our team to let them know, thanks. * Thread Starter [madri2](https://wordpress.org/support/users/madri2/) * (@madri2) * [11 years, 3 months ago](https://wordpress.org/support/topic/xss-1/#post-6043945) * any news ? * Plugin Author [JW Player](https://wordpress.org/support/users/longtail-video/) * (@longtail-video) * [11 years, 3 months ago](https://wordpress.org/support/topic/xss-1/#post-6043948) * I will need to ping them again. * [Zanes](https://wordpress.org/support/users/zanes/) * (@zanes) * [11 years, 3 months ago](https://wordpress.org/support/topic/xss-1/#post-6043965) * 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). * Plugin Author [JW Player](https://wordpress.org/support/users/longtail-video/) * (@longtail-video) * [11 years, 3 months ago](https://wordpress.org/support/topic/xss-1/#post-6043966) * Did you download 2.1.14? * [Zanes](https://wordpress.org/support/users/zanes/) * (@zanes) * [11 years, 3 months ago](https://wordpress.org/support/topic/xss-1/#post-6043967) * 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. * Plugin Author [JW Player](https://wordpress.org/support/users/longtail-video/) * (@longtail-video) * [11 years, 3 months ago](https://wordpress.org/support/topic/xss-1/#post-6043968) * We’re using a built in function from WP. * [http://codex.wordpress.org/Function_Reference/paginate_links](http://codex.wordpress.org/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](https://github.com/WordPress/WordPress/blob/4.2.2/wp-admin/includes/media.php) * May I ask why you think this is an issue? * [Zanes](https://wordpress.org/support/users/zanes/) * (@zanes) * [11 years, 3 months ago](https://wordpress.org/support/topic/xss-1/#post-6043969) * 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](https://github.com/WordPress/WordPress/blob/4.2.2/wp-admin/includes/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](https://make.wordpress.org/plugins/2015/04/20/fixing-add_query_arg-and-remove_query_arg-usage/) 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. * Plugin Author [JW Player](https://wordpress.org/support/users/longtail-video/) * (@longtail-video) * [11 years, 3 months ago](https://wordpress.org/support/topic/xss-1/#post-6043973) * 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. * ``` add_query_arg( 'paged', '%#%' ), 'format' => '', 'prev_text' => __('«'), 'next_text' => __('»'), 'total' => ceil($wp_query->found_posts / 10), 'current' => $q['paged'], )); if ( $page_links ) echo "
$page_links
"; ?> ``` * 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. * Plugin Author [JW Player](https://wordpress.org/support/users/longtail-video/) * (@longtail-video) * [11 years, 3 months ago](https://wordpress.org/support/topic/xss-1/#post-6043974) * 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#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#L2718) [https://github.com/WordPress/WordPress/blob/4.2.2/wp-includes/general-template.php#L2734](https://github.com/WordPress/WordPress/blob/4.2.2/wp-includes/general-template.php#L2734) * [Zanes](https://wordpress.org/support/users/zanes/) * (@zanes) * [11 years, 3 months ago](https://wordpress.org/support/topic/xss-1/#post-6043975) * 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](https://core.trac.wordpress.org/browser/tags/4.2.2/src/wp-includes/formatting.php#L3101) 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. * Plugin Author [JW Player](https://wordpress.org/support/users/longtail-video/) * (@longtail-video) * [11 years, 3 months ago](https://wordpress.org/support/topic/xss-1/#post-6043976) * It’s ok, thank you for being so thorough! 🙂 Viewing 14 replies - 1 through 14 (of 14 total) The topic ‘xss ?’ is closed to new replies. * ![](https://s.w.org/plugins/geopattern-icon/jw-player-plugin-for-wordpress_ffffff. svg) * [JW Player for Flash & HTML5 Video](https://wordpress.org/plugins/jw-player-plugin-for-wordpress/) * [Frequently Asked Questions](https://wordpress.org/plugins/jw-player-plugin-for-wordpress/#faq) * [Support Threads](https://wordpress.org/support/plugin/jw-player-plugin-for-wordpress/) * [Active Topics](https://wordpress.org/support/plugin/jw-player-plugin-for-wordpress/active/) * [Unresolved Topics](https://wordpress.org/support/plugin/jw-player-plugin-for-wordpress/unresolved/) * [Reviews](https://wordpress.org/support/plugin/jw-player-plugin-for-wordpress/reviews/) * 14 replies * 3 participants * Last reply from: [JW Player](https://wordpress.org/support/users/longtail-video/) * Last activity: [11 years, 3 months ago](https://wordpress.org/support/topic/xss-1/#post-6043976) * Status: not resolved