22 Comments

  1. Brin Wilson

    “…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. Sunil

    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

    • byheath

      “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

  3. Devin Walker

    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

    • Stephen Cronin

      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. Ray

    @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. Rick Dees (@r1ckd33zy)

    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. Jeffrey

    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

    • George Stephanis

      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. Kevin

    “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

  8. Gary Jones

    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

    • Gary Jones

      Oh, and please follow @TGMPA on Twitter for release notifications.

      Report

    • Jeff Chandler

      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

    • Stephen Cronin

      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

Comments are closed.

%d bloggers like this: