[wp-trac] [WordPress Trac] #9164: #6871 Regression for Plugin Dir
WordPress Trac
wp-trac at lists.automattic.com
Wed Feb 18 13:11:24 GMT 2009
#9164: #6871 Regression for Plugin Dir
--------------------------+-------------------------------------------------
Reporter: hakre | Owner: ryan
Type: defect (bug) | Status: new
Priority: high | Milestone: 2.7.2
Component: Security | Version:
Severity: normal | Keywords:
--------------------------+-------------------------------------------------
#6871 fixes a Bug that enabled a code-injection feature for wordpress.
Even the specific describben injection (in (#6871)) is fixed, it does not
fix an injection into an existing plugin dir.
There is not much strict information provided in the
[http://codex.wordpress.org/Writing_a_Plugin official docs], on how a
plugin must be designed. To fix this issue it is not only important to
have better checks in the code but to provide a strict definition in
parallel as a kind of contract for all plugin developers. This is why I
write some more words about the Issue.
To give an example:
By current defintion, a plugin must have a PHP-Comment containing
Information about the Plugin (called Plugin Headers). But the Function
validate_plugin() in /wordpress/wp-admin/includes/plugin.php does not
check this (Keep in mind that this matches even #6871 and renders it kinda
unfixed).
Additionally it is written, that you must/have to create a php file.
WordPress Coding Styleguide does not explicitly name it (does it?), but I
assume that for WordPress all PHP Files must have the file-suffix of .php.
This is something that could be checked against as well.
get_plugin_data() might come in handy to verify plugin metadata. at least
this looks like the undocumented but recommended API Function to read
Metadata out of a Wordpress Plugin. The Plugin Listing Page in the Admin
is using it.
In the Admin itself (/wordpress/wp-admin/plugins.php) the function
get_plugins() (/wordpress/wp-admin/includes/plugin.php) is used to
retrieve the list of plugins. But this Function is not used in the
validation function as well so that the listing becomes inconsistent with
the already coded protection against invalid plugins. I strongly encourage
to create more consistency here to gain an overall better stability. That
function for example checks for the '.php' file-ending. So I see no
problems to add it in the other check as well.
Anyway, this sole change won't help to create a better consistency here.
But it would be a start.
The main focus in my opinion should be put on the fact, that some plugins
that are active aren't in the listing of the actvive plugins. So the value
from the database can be validated against the value gathered in the
listing OR the other way round. For the listing, filesystem operations are
necessary (traversing directories), so the later would at least enable an
Admin to verify the current setup with lesser loss of performance.
It might be a good Idea to create check routine that is used on the Admin-
Plugin-Page (only) and those who want to make their site more secure can
use it in their Frontend as well.
So finally I would suggest to provide a better Check in the Admin-Plugin-
Page (add function validate_active_plugins_listing() that uses the same
code as the listing for the admin panel, maybe add another method for the
listing of plugins only first), add a .php file Ending Check in the
standard Plugin Check (function validate_plugin() in /wordpress/wp-
admin/includes/plugin.php).
--
Ticket URL: <http://core.trac.wordpress.org/ticket/9164>
WordPress Trac <http://trac.wordpress.org/>
WordPress blogging software
More information about the wp-trac
mailing list