[wp-trac] [WordPress Trac] #9164: #6871 Regression for Plugin Dir

[WordPress Trac]

#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