ktbartholomew / saml-20-single-sign-on

Wordpress plugin that makes a Wordpress site act as a SAML service provider
GNU General Public License v2.0
37 stars 22 forks source link

Plugin removed from directory due to security issue #20

Open ktbartholomew opened 7 years ago

ktbartholomew commented 7 years ago

Via email:

Hello,

Your plugin has had to be temporarily withdrawn from the WordPress.org Plugin Directory due to an exploit.

Plugin Page: https://wordpress.org/plugins/saml-20-single-sign-on/

Vulnerability Report: Your plugin is calling core loading files directly from files that are directly accessible.

Including wp-config.php, wp-blog-header.php, wp-load.php, or pretty much any other WordPress core file that you have to call directly via an include is not a good idea and we cannot approve a plugin that does so unless it has a very good reason to load the file(s). It is prone to failure since not all WordPress installs have the exact same file structure.

Usually plugins will include wp-config.php or wp-load.php in order to gain access to core WordPress functions, but there are much better ways to do this. It's best if you tie your processing functions (the ones that need but don't have access to core functions) into an action hook, such as "init" or "admin_init".

Please consult the Plugins API reference for more information: http://codex.wordpress.org/Plugin_API

If you're trying to use AJAX, please read this: http://codex.wordpress.org/AJAX_in_Plugins

For other possibilities, or to better understand why we disallow this, read this: http://ottopress.com/2010/dont-include-wp-load-please/

If you're trying to use it because you need to access WordPress functions outside of WordPress, we'd actually much rather you didn't do that at all. Your plugin should be inside WordPress, only accessible to people who are logged in and authorized, if it needs that kind of access. Your plugin's pages should be called via the dashboard like all the other settings panels, and in that way, they'll always have access to WordPress functions.

Plugins are closed immediately, once we are made aware of any security issues and have verified their exploit-ability. We would be putting users at risk if we allowed them to download code that could be exploited, and once an exploit is reported, it is often acted upon by persons nefarious. As we cannot guarantee the author(s) have been contacted, due diligence demands we immediately close the plugin and contact the author. This action is applied to all plugins hosted in the WordPress directory. As soon as a fix is committed, the plugin can be checked and re-opened. This timeframe for this is entirely dependent on the speed of response.

Please review the exploit report carefully. If you believe the report is NOT valid, and that your plugin is secure, please reply to this email to let us know. If the vulnerability is XSS or CSRF related, know that Chrome actually prevents those from working in their browser and you'll need to check in Firefox.

If you find this report to be valid, you must close the exploit and update your plugin in our director (via SVN) in order to have it restored, in order to protect your user base. When you do so, we ask you increase the version number of your plugin to reflect the change, to ensure that blog owners are alerted to update. Should you, for any reason, find you are unable to update the plugin, please let us know promptly so we can decide on the action to take to best look after the plugin users.

Your plugin will not be re-opened until it is reviewed, and it won't be reviewed until you reply to this email, so please do so as soon as you've corrected the issue and checked the new code into SVN. This review process may take a while. Please be patient. While we fully understand that your plugin is important to you, it can take us up to 5 business days to give your plugin a full review. We check both your changes and your plugin as a whole, to ensure we didn't miss anything.

If you have any queries or need any advice, do get in touch.

Thank you

ktbartholomew commented 7 years ago

It seems they're taking issue with lines like https://github.com/ktbartholomew/saml-20-single-sign-on/blob/master/src/saml/www/_include.php#L25 and would prefer that we lean on built-in WordPress actions to then trigger our behaviors.

It looks like the init action fires early enough for the plugin to intercept the usual WordPress goings-on and either respond with XML metadata or process a SAML assertion (basically the only two things for which we use SimplSAMLPHP directly). There are a few challenges in switching to this way of doing things:

  1. How does the plugin identify when a request should be intercepted and used to either display SP metadata or consume a SAML assertion? It would be nice to have pretty URLs like /saml/acs or something, but that could very easily conflict with existing permalinks or a site that simply can't/doesn't use permalinks. We could also use a query string like ?saml_action=get_metadata that could be appended to any URL. Not sure if some IDPs would have a problem doing an HTTP redirect binding to an endpoint that requires a query string. How we do this could also be user-configurable if no single way is win-win.
  2. How do we make SimpleSAMLPHP behave correctly when it's not being called directly? Most likely we're going to need to solve https://github.com/ktbartholomew/saml-20-single-sign-on/issues/10 as a means to get this fixed.
ktbartholomew commented 7 years ago

Also, since this will drastically change the way users and IDPs interact with sites using this plugin, fixing this will probably necessitate a major revision bump.

khromov commented 7 years ago

@ktbartholomew Just started looking at this plugin, but if you're after a custom endpoint, perhaps the REST API would work?

https://developer.wordpress.org/rest-api/extending-the-rest-api/adding-custom-endpoints/

jeremykenedy commented 7 years ago

@ktbartholomew Any word/eta on having this back into the WP registry?

ktbartholomew commented 7 years ago

@jeremykenedy https://github.com/ktbartholomew/saml-20-single-sign-on/pull/21 aims to resolve the things that the WP registry has taken issue with. Unfortunately, there's still a lot of work left to be done before that pull request can be shipped. The best ETA I can provide is "when I have time".