My plugin removed from WP.org extend directory

[Jeffro]

Based on what I see here, this is a bad way of handling the situation. You should be told just as WordPress does explicitly what the security implications are and if possible, a fix to those security concerns. Instead, you've been told that he said she said and the plugin has security concerns. If the plugin truly has a security vulnerability then it's good that it was taken down but to not tell the plugin author what the vulnerability is or what to look for is pretty bad.

[chipbennett]

Based on what I see here, this is a bad way of handling the situation. You should be told just as WordPress does explicitly what the security implications are and if possible, a fix to those security concerns. Instead, you've been told that he said she said and the plugin has security concerns. If the plugin truly has a security vulnerability then it's good that it was taken down but to not tell the plugin author what the vulnerability is or what to look for is pretty bad.

That's why I said that it smells fishy.

The plugin shouldn't be removed until specific security vulnerabilities are divulged to both the repo moderator (MarkR) and the plugin author (Alden).

And the plugin author should be given the opportunity to patch the specific security vulnerabilities divulged, before the plugin is removed.

Perhaps it is merely a coincidence that the plugin in question is a fork of the single most-popular plugin in the repo - a plugin that has (again, coincidentally) just been moved to Pro (i.e. paid) status. Perhaps it is merely a coincidence that an (unnamed) dev just happened to peruse this particular plugin, and find (unnamed) security vulnerabilities.

But then, that's all the more reason to ensure that the situation is handled appropriately - because, even if everything is coincidental, the way it is handled can give the appearance/perception of impropriety.

[Ryan]

This seems rather odd. You'd think if they found a security vulnerability the first reaction would be to remove the plugin temporarily, contact the owner of the plugin immediately via email explaining the nature of the problem, then once it's sorted out add it back again.

This current situation is just plain bizarre.

I can't imagine this is standard procedure for the repository. This just seems like a muckup in the system somewhere.

[chipbennett]

This seems rather odd. You'd think if they found a security vulnerability the first reaction would be to remove the plugin temporarily, contact the owner of the plugin immediately via email explaining the nature of the problem, then once it's sorted out add it back again.

This current situation is just plain bizarre.

I can't imagine this is standard procedure for the repository. This just seems like a muckup in the system somewhere.

I'm glad to see that I'm not the only one who sees something amiss here...

[andreasnrb]

Ok I had look on the plugin. Can't understand where the PHP learner thing comes from.
Ain't much security problems to talk of either I think but ain't 100%. The ones I found serious are those unescaped uses of $_SERVER that are combined with other strings and then echoed/returned. They only happen on 404s.
There are far far far worse cases in the directory.
Here comes overall plugin suggestions.

wp_head()
Line 378: Use esc_attr($description)
Line 390: Use esc_attr($keywords)
Line 470: Use esc_url($url)
Line 695: esc_url($_SERVER['REQUEST_URI'])
Line 696: see above
Line 942: see above
Line 943: see above


management_panel()
You use htmlspecialchars and stripcslashes so thats good.
You use form action="" you should use <form action="<?php echo esc_url($_SERVER['REQUEST_URI']) ?>">. A Safari thing I think. You could also make use of options.php makes things easier. (See bottom).

Some improvements
You have a lot of calls to specialchars, stripcslashes etc. No need to use them until you actually return/echoes the strings. I at least call escaping last so I can easily see that I've done it. Also perhaps extract some variable settings outside all your if statements. Got a lot of code that is in every if statement.

• get_url(). It ain't used.
• management_panel(). Aint used
• Line 276: You never use $title anywhere in the function.
• On options you can make use of the fact that options.php handles options array. much less code required. See this link: Handling Plugins Options in WordPress 2.8 with register_setting

[aldenml]

Hi andreasnrb, I'm glad you spent some time looking at the source code and I really appreciate your comments. Some of them apply directly to the original plugin (AOSEOP) since it's a fork.

One of the main objectives is perform all this obvious and no so obvious improvements taking care of readability and documentation in order to make it easy to modify for another programmer. Also, extensibility by hook functions is another key point. I'm writing down your suggestions and I will give to you the credit for such improvements in the change log (unless you don't want it).

I think the "PHP learner thing" is very unfortunate since in the programming world, we are always learning. Some knowledge are universal, no matter what platform, framework or programming language is. Additionally I strongly believe that in the Open Source area, it's totally pointless make a critics, ban or forbid another work without giving your contribution.

I don't know how the MarkR's responsibility really operate, if he is elected in some committee or if he receiving instructions from others. I just wish that he could provide me with more information to work with and solve the problem as soon as possible.

[andreasnrb]

Hi andreasnrb, I'm glad you spent some time looking at the source code and I really appreciate your comments. Some of them apply directly to the original plugin (AOSEOP) since it's a fork.

Your welcome. I think I covered all code and I couldn't find anything special. Maybe the unknown dev can show up and let us know. Cause I would like to know if I missed something. If I did I've perhaps done the same mistake myself. Perhaps there is a problem with the query calls but you used prepare etc.

I'm writing down your suggestions and I will give to you the credit for such improvements in the change log (unless you don't want it).

Credit is always nice. Good for boosting the ego. Hope I my suggestions are correct hehe.

I don't know how the MarkR's responsibility really operate, if he is elected in some committee or if he receiving instructions from others. I just wish that he could provide me with more information to work with and solve the problem as soon as possible.

Well the wp organization works in mysterious ways. I call it a black box with a magic unattached button =). And referring to an anonymous source and not reveal what the problem is. Thats just bad manners. Not exactly living up to the "spirit" of the open source community.

PS
Mark J got a good post on the esc functions Escaping API updates for WordPress 2.8
Also a good read http://codex.wordpress.org/Data_Validation

[Ryan]

I'm glad to see that I'm not the only one who sees something amiss here...

I see something amiss, but I wouldn't use the term "fishy" for it, since I can't see any reason why there would be. It's just looks like a accidental balls up by someone somewhere along the line.

[aldenml]

An update.

I contacted Mark Jaquith. He is one of the WordPress Lead Developers. We had a brief, interesting and extremely helpful (for me) conversation. He was open to take a deeper look to the plugin.

Now I need to be patient. I'm sure Mark Jaquith will find the big security issue (if any) or conclude that the security level is similar to the original plugin All in One SEO Pack.

My plans are continue the technical discussion until I got my plugin included in the extend again and research what was behind the way MarkR conducted the decision, since everybody agrees that there is something wrong on it.

I'm sure something needs to be improved in the WP.org side. Take a look at this. Someone wrote in the WP.org forum that the typical "add it" (http://wordpress.org/extend/plugins/add/) form can help you to send request information. I followed the advice with this values:

Plugin Name (required): Light SEO Already Created
Plugin Description (required): {A brief description about my situation}

Guess what, I have a new plugin approved:

Email subject: [WordPress.org Plugins] Request Approved: light-seo-already-created
SVN repository: http://plugins.svn.wordpress.org/lig...ready-created/

The approval was in less than two days.

[Ryan]

It's good to see someone higher up taking a look at this. Hopefully Mark can find whatever security flaw it was that had you kicked off the repository, or at least confirm that there isn't one.