XSS Vulnerability: What to do if You Buy or Sell Items on Themeforest and CodeCanyon

Important Featured Image
photo credit: What’s important?(license)

Earlier this week, one of the largest coordinated efforts between WordPress plugin authors, Sucuri, and the WordPress security team resulted in a number of popular plugins receiving security updates. Due to inaccurate information within the WordPress codex, a number of developers improperly assumed the add_query_arg() and remove_query_arg() functions would properly escape user input.

When combined, Themeforest and CodeCanyon sell nearly 8.8K WordPress items. Stephen Cronin, Quality Team Leader for Themeforest and CodeCanyon, has published an official forum post that describes the vulnerability and how sellers can check for it within their items. If items you sell use the following code, it is likely affected.

  • add_query_arg()
  • remove_query_arg()
  • TGM Plugin Activation class

TGM Plugin Activation is a PHP library created and maintained by Thomas Griffin and Gary Jones that allows developers to require or recommend plugins for themes or for plugins. It allows users to install and even automatically activate plugins in singular or bulk fashion using native WordPress classes, functions, and interfaces. Sellers should review their code and follow the guidelines published on the Make WordPress plugins site.

While auditing the TGM Plugin Activation class, a XSS vulnerability was discovered. The TGM Plugin Activation class has since been updated despite the version number not being changed. If you’re a seller and use this class, you’ll need to update to the latest version of TGM Plugin Activation and update your item to include the latest version.

OptionTree

If you use OptionTree, the marketplace review team is confident that all instances of add_query_arg and remove_query_arg have been escaped properly. There will be an update in the future that escapes these functions you should include in your item, but you shouldn’t delay updating your items while waiting for the update.

Redux Framework

The Redux framework also uses add_query_arg and remove_query_arg, but most are escaped appropriately. There are a few questionable areas within the theme that the review team will provide updates on once they receive clarification.

Theme Authors

Theme authors who have bundled affected third-party plugins will be contacted by Envato in the next few days to update your theme. You’re encouraged to check bundled plugins before this time to see if they’re affected.

Buyers

According to Cronin, all WordPress specific items are being evaluated. Once the evaluation is complete, buyers who purchased an affected item will be notified. There’s no time frame on when the evaluation will be completed, however, Cronin says it is a priority and progress reports will be published in this forum thread.

All Hands on Deck

Cronin says, “When submitting an update that addresses these issues, please include a note mentioning it’s related to the XSS vulnerability. This will allow us to prioritize the review of updates.”

Unlike the WordPress.org plugin directory, Themeforest and CodeCanyon only provide and notify buyers of updates if they register with the update system. It’s not an optimal upgrade routine and one that requires buyers to opt-in instead of opt-out.

It’s important that sellers on Envato’s marketplaces do their part to check and patch any XSS vulnerabilities discovered. It’s also important the lines of communication remain open between the marketplaces and buyers so they can update purchased items as soon as possible. If you do business with Themeforest or CodeCanyon, be on the lookout for updates to items you’ve purchased.

22 Comments


  1. “…only provide and notify buyers of updates if they register with the update system”

    – I’ve never heard of the update system before… how does one opt-in?

    Report


  2. Does anyone know if there is tooling that can scan sites across for this specific vulnerability?

    I guess such a tool would basically grep across the webroots of all vhosts for occurences add_query_arg() and remove_query_arg(), and then testing to see if input is sanitized.

    Given the co-ordinated response from various vendors and WordPress sec team I can imagine that the expertise exists to produce a tool that can help us check our sites.

    Report


    1. “We are currently evaluating all WordPress items sold through Envato Market.”
      …TGM is a required plugin for all ThemeForest themes… so most/all up to date themes sold on themeforest will require the TGM update.
      Mine included. Easy peasy tho :)

      Report


      1. Hey Heath,

        TGM is a required plugin for all ThemeForest themes

        Well, it’s not required for all TF themes, only those that are including plugins. It’s fair to say that a LOT of them do use it though… :)

        Report


  3. Sadly I doubt many less popular plugins on these market places will bother to be updated. I hope the marketplaces themselves evaluate the code bases and provide the developers with a window to patch or else it gets taken off their market. That seems like the most logical way to go about it from what I can see.

    Report


    1. Hey Devin,

      When we find an item with a serious security vulnerability, we disable it straight away (as we did with hundreds of themes in the Rev Slider case). For less severe vulnerabilities, we generally ask the author to fix it and give them a window, after which we will disable their item if not fixed.

      This one is a little different, in that it’s wide ranging and also the presence of the 2 functions doesn’t necessarily mean that the item is vulnerable. It needs to be checked by the author and updated if necessary, but we can’t automatically assume there is a problem – so we won’t be disabling items at this point (any more than the .org team are disabling plugins there).

      We will be emailing all authors with items that contain one or both of the functions (whether in their code or TGM PA or other) and asking them to check their code and update asap. We’ve already posted on the forums and we’ll be linking to that from the author dashboard – so at least we think we’ll reach most authors.

      Report


  4. @Devin Walker – agreed.

    I think things are improving as plugin and theme authors are becoming more aware of security considerations, however, some of the less profitable ones will simply not spend the time to update their code.

    Of course, none of the efforts to inform theme & plugin authors to get them to update will count for anything if there are still lots of businesses who don’t value regular maintenance on their websites (whether by themselves or a third-party). There needs to be equal effort spent in this direction too.

    So, battle is really on two fronts whenever vulnerabilities such as this are discovered:
    1). Getting theme and plugin authors to update their code
    2). Then communicating to businesses and web admins to help them see the value in keeping their websites up to date.

    Report


  5. If I run ‘grep -Hnrw add_query_arg .’ from my plugins folder I am getting lots of hits. I am thinking this is search pattern is too broad. Does anyone here have any suggestion on refining the grep pattern?

    Report


  6. Forgive my ignorance, but would it be easier if add_query_arg() and remove_query_arg() functions are modified in the core to properly escape user input then do a core security update?

    Report


    1. It doesn’t work that way, as folks can also use the returned url in a Location header or the like, that would not accept a HTML escaped version.

      It was a good idea, but we’d already thought it through and sadly no go.

      Report


  7. “Due to inaccurate information within the WordPress codex, a number of developers improperly assumed the add_query_arg() and remove_query_arg() functions would properly escape user input.”

    How exactly was the codex wrong? Just because it didn’t say to use esc_url does it mean that it was wrong? There are hundreds of examples in WordPress that use esc_url and its been a common practice in themes and plugins for quite some time.

    Report


    1. There were a number of examples without escaping it and some confusing verbiage, I believe. As it is a wiki, you should be able to browse the history.

      Report


      1. I was able to see a past version and you are correct.

        Regardless its always a good idea to understand the methods you are using in your code and not to just blindly trust them.

        Report


  8. I’m one of the developers of TGMPA.

    > The TGM Plugin Activation class has since been updated despite the version number not being changed.

    That’s misleading, but it would explain the wording in GitHub Issues we’ve been getting.

    The currently tagged release is 2.4.0 and has been for over a year. As per typical git-flow development, we’ve got a hotfix/2.4.1 branch which should be merged with the master branch, tagged and released within 12 hours from now. It’s at that point we’d recommend theme and plugin developers update.

    The confusion is that we’ve also already merged the changes (including version number bump) into the develop branch, and at the same time, switched the default branch on GitHub to be develop – this is the branch you see when you look at the repo on GitHub.

    The master branch also erroneously had commits made since 2.4.0 was released. These have been replicated in the develop branch, and the master branch will be reset to the 2.4.0 commit before the 2.4.1 merge.

    The one main erroneous commit was regarding the removal of the deprecated screen_icon() function. This will be fixed, along with a load of other improvements, in a tagged 2.5.0 release which is due sometime in May, when it’s ready.

    As always, just like WPORG plugins, we recommended only including code from tagged TGMPA releases in your theme and plugin releases. We do appreciate the bug reports and improvement suggestions from those who use the develop branch.

    Report


    1. Thanks for the update and explanation as to what’s going on. The information you quoted was taken from the initial post on CodeCanyon.

      Report


      1. I’ve pushed a tagged 2.4.1 release that folks can download and use when updating their bundled TGMPA packages with their themes. It is available now in Releases on Github. :-)

        Report


    2. Hey Gary,

      Apologies for any confusion we’ve caused – we were getting pretty confused ourselves!

      The initial pull request to fix this was merged into master and that was the default branch, so that’s what we were telling people to use. Of course it wasn’t actually a release, so the version number stayed the same, hence the wording we used.

      I did ask Thomas if we could get the version number changed and submitted a pull request bumping the version after a brief Twitter conversation. Of course, that was based on the initial pull request, which turned out to break things, so was never accepted.

      At the time we wrote the post, that was the best wording we could come up with! In hindsight, I should have spent longer talking to you to find out your plan (at the time I had about a dozen balls in the air). Sorry again for any confusion caused.

      Anyway, we’ll direct theme authors to the tagged 2.4.1 release now that it exists (Thanks Thomas!). It looks like that also fixes the bulk install bug, which is great!

      Thanks for getting all this sorted out. :-)

      Report


      1. Sorry Stephen, but hold off on that – I’ve just had to delete that tag (a bad thing, I know), since unfortunately it was done wrong (whilst I was sleeping). I’ll be handling it properly later this morning.

        Report

Comments are closed.