Zanes

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.

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.

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.

Hi,

It’s up to each developer to ensure that their plugins are properly coded.
There’s unfortunately no magic for this, as the WordPress Team cannot see every vulnerability on all the plugins (and there are a lot of them!).

There are plugins to harden your site against such attacks though. But 100% prevention is never possible.

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).

Hi again,

Thanks for your reply.
Tried turning on the need_refresh, but the problem is that it disables the ability to get “older comments” (ie : the pagination links do not show) in the widget (I use the widget). I’ve had this problem when I first tried to tweak the code some time ago, and I haven’t fixed it (yet ?? ).

When you say “not” recommended, do you mean it’s because the server will have to make a request to the database each time somebody loads a page with the widget? I know this is kind of inevitable, but can’t help it :/
Also, I believe you only do a “get older comments” request when the pagination link is clicked, and not at the start, right? (not really sure about that one)

Regards,

Zanes

Hello, sorry for the late reply.
I don’t have an account on Github (got my own SVN), so i’ll paste what you need to get here. It’s quick and dirty, so it’ll probably need some optimization. Here’s the code (I believe you’ll know where this is located) :

/**
* Format comment contents
*/
	function format_comment($commentdata)
	{
		$comment = array();

		$comment['comment_number'] = isset($commentdata['comment_number']) ? $commentdata['comment_number'] : '';
		$comment['pid'] = $commentdata['comment_post_ID'];
		$comment['home'] = get_option('home');
		$comment['user_id'] = $commentdata['user_id'];
		$date_format = (!empty($this->options['input_date'])) ? $this->options['input_date'] : $this->options_default['input_date'];
		//$comment['time'] = date_i18n($date_format, strtotime($commentdata['comment_date']));

		  $difference = time() - strtotime(get_gmt_from_date($commentdata['comment_date']));

		  $weeks = round($difference / 604800);
		  $difference = $difference % 604800;
		  $days = round($difference / 86400);
		  $difference = $difference % 86400;
		  $hours = round($difference / 3600);
		  $difference = $difference % 3600;
		  $minutes = round($difference / 60);
		  $difference = $difference % 60;
		  $seconds = $difference;

		  if ($weeks > 0) {
				if ($weeks == 1) {
				$comment['time'] = $weeks . ' ' . __('week', $this->_folder); }
				else { $comment['time'] = $weeks . ' ' . __('weeks', $this->_folder); }
		  } else if ($days > 0) {
				if ($days == 1) {
				$comment['time'] = $days . ' ' . __('day', $this->_folder); }
				else { $comment['time'] = $days . ' ' . __('days', $this->_folder); }
		  } else if ($hours > 0) {
				if ($hours == 1) {
				$comment['time'] = $hours . ' ' . __('hour', $this->_folder); }
				else { $comment['time'] = $hours . ' ' . __('hours', $this->_folder); }
		  } else if ($minutes > 0) {
				if ($minutes == 1) {
				$comment['time'] = $minutes . ' ' . __('minute', $this->_folder); }
				else { $comment['time'] = $minutes . ' ' . __('minutes', $this->_folder); }
		  } else if ($seconds >= 0) {
				if ($seconds <= 1) {
				$comment['time'] = $seconds . ' ' . __('second', $this->_folder); }
				else { $comment['time'] = $seconds . ' ' . __('seconds', $this->_folder); }
		  }

		//$comment['time'] = get_time_diff($commentdata['comment_date']);
		//$comment['time'] = apply_filters('bwp_rc_date_format', $comment['time'], $commentdata['comment_date']);
		// for dynamic comment_type loading
		$comment['comment_type'] = $commentdata['comment_type'];
		$comment['type']	= $commentdata['comment_type'];
		// Format post_title, also trim it if necessary - @since 1.1.0

I’ve commented the parts in your code that aren’t used anymore.

For this to work neatly, the cache needs to be disabled. Adding an option to turn off the cache would be nice (it’d need to be activated by default if this procedure is used though) ??
Do you have any way to quickly turn off the cache in your code? I’d love to do a quick fix in the code for my website.

Regards,

Zanes

Had to change mine to
echo "<span id='$span_id-time'>" . (date_i18n('U')-2*60*60) . '</span>';
to match my blog’s time.

Looks like the plugin doesn’t seek the right time automatically :/
Worked fine in the previous version though.

Thanks for your answer ?? Hopefully this should help other people.

Hi again,

I think one part of the reason it won’t work is because on my Settings -> Permalinks, I have selected :
?Day and name h.ttp://example.com/2011/07/28/sample-post/
(notice the trailing slash at the end of the permalink structure)

Your plugin can’t seem to take me to the management page because it uses sample-post/?srp=XXXX instead of sample-post?srp=XXXX

I could probably fix this by using a custom permalink structure without a trailing slash at the end, although that’s a bit of an overkill ??

Management page works fine with the virtual page enabled, so I guess I’ll stick with this.
Was the request supposed to work with ___/?srp=XXXX or does it only work when there is no trailing slash (ie : ___?srp=XXXX)?

About the “replies only” : Ok my bad, I thought it was only for the admin (thus he should’ve had all options) and not for authors ??

Regards,

Zanes

(PS : For your request, sorry I can’t :/ security oblige)

Hi camu, and thanks for your reply.

I already tried to add a trailing slash to the management page, however that doesn’t work :/
I do arrive to the custom page I created to manage the subscriptions (copy of page-noside-nocomment.php from the Monochrome Theme), but what I have are the same options that appear at the bottom of a comment box : the dropdown menu, the text which says if you’ve subscribed or not, and if I can manage the subscriptions for this entry (ie : the same behavior as a post).

For the “replies only” i mean the link below the comment box in a single post : “You can manage the subscriptions of this entry.”, which brings me to a page with a list of all post’s subscribers, and some radiobuttons to change their status (active, suspended, etc). And here, the radiobutton to set their subscription to “replies only” is missing (well at least it’s missing for me). Subscribers correctly have this option though.

Regards,

Zanes

Désolé jeff_ ^^

Finally, I got it.

Images wouldn’t show because I added :
add_theme_support( ‘post-thumbnails’ );
to the functions.php of my theme (as I said in my first post).

I removed it to get my old functions.php back, and now it is working.

No idea why adding this function to the theme breaks your plugin.. 50% chance that it comes from the theme. No idea :/

Works fine now ! Thanks for your time jeff_

By the way jeff_, just wondering :

Does your plugin requires to connect to a website before working? Because when I manipulate plugins, I put my blog into maintenance, and nobody except me can reach the blog (all other people are redirected with a htaccess).
Since I need to turn on the maintenance before mainpulating the plugin, could this be a reason it is not initializing? (not talking about the “images not showing” problem of course).

Hmm doesn’t look good. Deactivated all my plugins (problem was still here), then the slideshow.
Now, when activating the slideshow again, I can’t see anything in both the admin area and outside.
The do_shortcode is executed, and I can see the output in the source code of the page, but nothing appears on screen.
Either the plugin thinks it has already initialized or maybe my database got corrupted when I deactivated/reactivated the plugin, I don’t know ??

Even my test blog (which has a separate database), doesn’t work now, and I didn’t even touch anything.. wow ??

May be coming from my webhost, but I wouldn’t know why (yet).

Oh well, the slideshow does work in my test blog.. It may be another of my plugins (on my main blog) which is conflicting with yours :/
I’ll try to figure it out.