Cancel

Jeff Chandler

User Role Editor 4.25 Patches Critical Security Vulnerability

Vladimir Garagulya, developer of the User Role Editor has patched a critical security vulnerability. User Role Editor is used to edit, manage, and create user roles and capabilities and is active on more than 300K sites.

User Role Editor Interface
User Role Editor Interface

User Role Editor 4.24 and below allows any registered user to gain administrator access. Wordfence, a popular security plugin for WordPress has more details and explains why the plugin was vulnerable:

The author was checking if users have access to edit another user using the ‘current_user_can’ function and checking for the ‘edit_user’ (without an ‘s’ on the end) capability on a specific user ID. A user can edit themselves, and so sending data to the plugin that supplies the current user’s ID to this access check would bypass the check.

Users should immediately upgrade to 4.25 to protect sites from this vulnerability.

Category:
Tags: ,
45

45 responses to “User Role Editor 4.25 Patches Critical Security Vulnerability”

      • This is a very nasty one. If you know your users, there is probably not much to worry about, but if your site is open to all to register, then all you can do is cross your fingers and hope. Smart hacker will not leave any obvious foot prints and you will have to audit everything on your site. For example they might have upgraded the user to “admin”, used the plugin editor to add some backdoor code into one of the plugins and then reverted the user to the original state.

        Create forum topic from comment

        • then all you can do is cross your fingers and hope

          Well, you can audit all your privileged users, and your code (and everything else, depending on your risk assessment). I notice you’ve slightly walked that back, by saying that you can audit.

          We have tools that checks code against pristine copies. I was hoping Vladimir, as a service to his users, might have a fragment for auditing the users – if not, it’s a few hours work (or Google first to see who else has done it!).

          Apologies to Jeff – I mistakenly clicked “Report” instead of “Reply” before typing this. I don’t seem to be able to “Unreport”.

          Create forum topic from comment

        • Here’s a quick-and-dirty script… note that a) it’ll need adapting to fit situation of the site b) it relies upon having already audited that there are no unexpected users outside of the ‘customer’ role, and that you don’t have hacked code that might give false results (or have copied your user tables to a clean WP install). c) I don’t have the resources to give any help with customising it.

          This forum doesn’t like direct code posts; so, you’ll need to download it from here: http://david.dw-perspective.org.uk/tmp/audit-users.txt

          Create forum topic from comment

  2. This is maddening…..

    Incompetent plugin developers (screwed this up by missing an ‘s’ in the function name – seriously?) playing with things that might better be solved in WP core.

    Using an undocumented function without knowing how to use it is ridiculous (or in this case not knowing how to call it properly).

    This plugin is deleted from the sites I operate that use it and is now officially dead to me. I can’t risk my clients sites over incompetence of a plugin dev.

    Create forum topic from comment

    • hmmm, the plugin author didn’t force you to install it. You haven’t paid for it, you haven’t vetted it out in any way before installing it on live sites, but still it is somehow the plugin author’s problem.

      No, it is actually your problem, and as long as you will continue to no vet whatever you install on your sites this will continue to happen to you.

      Create forum topic from comment

      • As an end user it is in NO WAY my responsibility to “vet” a plugin – free or otherwise.

        It is the author’s responsibility to ENSURE that there are at the very least NO SECURITY VULNERABILITIES.

        WP should remove this plugin immediately as this author has proven that he can’t VET his own plugin for something as basic as this.

        Create forum topic from comment

        • Well, if you don’t care about your sites that much, then there is no reason that you will vet anything. I just wonder why are you getting so upset about something you don’t care much about.

          BTW, do you understand why the current wordpress version is 4.4.2? I assume you didn’t stop using wordpress because of its security issues.

          It is perfectly fine to be upset about something like this that should never happen in a properly tested piece of code, but your response is more like a kids tantrum than a “grown up” way to handle such issues. Bugs happen, security problems happen, even end users usually understand that.
          The best approach to security is to assume there are security issues with every software and build your system in a way that both will minimize the chance that anyone will be able to take advantage of them, and have an easy to implement recovery plan if it happens

          Create forum topic from comment

        • All software has bugs. What ever operating system you are using to post here has had security exploits at one time. Or have you switched to your own operating system? Your ignorance is amazing.

          Create forum topic from comment

    • Not to sound like a jerk, but if that is the mentality that you use, you might want to start uninstalling everything you use. Thinking that any piece of software would be perfect is ridiculous.

      I would understand your reaction if you had to uninstall the plugins because of patch that was going unfixed. The fact is that you didn’t. I would guess that you haven’t had any issues relating to this issue.

      No I would say that you wanted to attack a hard working plugin author, who provides a plugin for free. One who has already fixed the issue. If anything, you should be thanking the developer. Especially if you haven’t had any issues relating to this, I’d say that he saved you a hell of a lot of trouble in the long run. I’d rather update a plugin any day than deal with the issues that came about from anyone being able to access the admin area.

      Create forum topic from comment

      • It should have never been allowed to happen in the first place. It was a FREAKING SPELLING ISSUE!!!

        I don’t “thank” free plugin developers. It’s their choice to do that – not mine. Plugin devs are a dime a dozen. Who cares?

        Of course this is why I actually prefer to pay for good plugins most of the time – lesson learned on this one.

        Usually it’s simply a case of the plugin not being good enough to actually charge for (as in this case obviously).

        Create forum topic from comment

        • I agree, often times paid plugins are better. But I there are A LOT of really amazing free plugins.

          Just because someone doesn’t choose to charge for something doesn’t mean it’s not a quality product. I can name a TON of examples. WordPress for one. There are some people that actually like to release their plugins for free because that’s what they believe. It doesn’t in NO WAY make them any less of developer. In fact, I have an enormous respect for those developers. A lot of paid plugins are absolutely no better than the free ones as far as quality of code goes. Just because you can charge for something doesn’t mean you should. Just because you should charge for something doesn’t mean you have too.

          And you said it yourself, it was a spelling mistake. I don’t know about you, but I’ve definitely made my fair share of mistakes in life. A quick look at this plugin and you will realize that there are THOUSANDS of lines of code in the plugin. That is no joke.

          The fact is that every piece of software has bug, issues, and security vulnerabilities. And I’m sure that A LOT of them have spelling mistakes. It’s not big deal until they are discovered. It’s about how the patches are provided that matters. In this case, everything turned out fine.

          We are humans. Mistakes happen. It’s about fixing our mistakes and learning from them. No one is perfect and no software is perfect.

          Create forum topic from comment

  3. Ron,

    You’re trying to look smart but you’re just embarrassing yourself.
    This is not a function and it’s not a typo.

    Those are roles and capabilities and both “edit_user” and “edit_users” are valid in different contexts.

    Read more on: https://codex.wordpress.org/Roles_and_Capabilities#edit_users

    Do you know how many hours developers put into plugin development (for free) ?
    I bet Vladimir has put at least 250-500 hours into this plugin.

    I also bet that in you’ll reinstall the plugin in the next 3 months.

    Create forum topic from comment

    • “Embarassing myself” means that I care what you or anyone else here thinks.

      NEWSFLASH – I DON’T.

      You’re the one that is WRONG though. There is not one single reference to edit_user in the link you provided.

      Now who’s “embarrassed” comrade?

      Again, I don’t care about the “free” part. Dime a dozen…

      Create forum topic from comment

  4. Ron is obviously being provocative deliberately. The interesting thing is the sort of response that he is provoking. Those responses (with the exception of that of mark k) highlight the major failing of the WordPress eco-system.

    The inevitable response to a major error in any other walk of life would be to ask what we can learn from it. Of course humans make mistakes, so the question becomes what we can do in future to minimize the chance of a recurrence and/or catch them before they do any harm.

    In the WordPress world, unfortunately, the dominant response is either “Well, what do you expect?” or “You should be grateful you got something for free!” Sometimes they even get combined into “Be grateful you got something for free, even though it’s bound to be buggy!”

    I had assumed that developers like to think of themselves as professionals. Yet we don’t expect doctors or engineers to react like that. They are still humans, and they still make mistakes, but systems and procedures are set up to limit their occurrence and any harm they might do.

    So where are the proposals for establishing systems to try to reduce the likelihood of a recurrence of what happened here? It was, after all, exactly what Ron has described: a spelling mistake. Despite what Slavi Marnov says, the plugin developer was not trying to target the “edit_user” capability at all. He meant “edit_users” and just misspelled it.

    Surely the first problem that’s apparent here is that it is crazy to have two such different capabilities present in WordPress when the only thing that differentiates them is one “s”. That’s just asking for trouble. If “edit_user” had been “edit_member”, for example, then the spelling mistake would have been noticed because the plugin just wouldn’t have worked. That’s a simple fix that should be implemented asap.

    But how many other such bear-traps are lurking? And what testing protocols should be developed and published? mark k is right that everyone needs to vet his/her own sites, but without some sort of checklist, that doesn’t amount to much.

    So please stop responding to Ron as though there’s nothing that could have been/can be done, and let’s see some practical suggestions on how to improve things in future.

    Create forum topic from comment

    • Unfortunately, under proper software development established practices this specific bug, would have been easily detected as part of the QA, specifically any unit testing that made sure that all code paths were tested.

      The problem is that the wordpress community is so used to hack solutions instead of developing software it is pointless to point at one developer for not doing what most likely no one does (free or paid, I also laugh at Ron’s trust of paid plugins, in my experience they are actually worse).

      What can be done? Each plugin and theme should be demanded to show its testing plan, hopefully as a phpunit tests so you will be able to test it fully on your site instead of just randomly check some features.
      In addition wordpress.org can run some code complexity analyzer and give plugins marks based on their complexity. Usually the more complex a piece of code is the harder it is to actually test all its execution paths.

      But in the end this comes back to money. This plugin according to its site has 1400 which are 0.5% paying users. with 30$ per user, this is not great but ok and should be enough to get someone to do proper QA. So what the paying users should do is to ask for their money back as the product was obviously defective in a relatively easy to detect way, and the cost of figuring out the implications of that security problems will be much bigger then the cost of the license. Once people will start demanding money back maybe plugin and theme authors will upgrade their game.

      Create forum topic from comment

    • What evidence do you have that the plugin author was trying to target edit_users but mistakenly targeted edit_user?

      It is clear that he meant to use the edit_user capability, but was just unclear about its ability for that to always return true so a user can edit itself. This is because providing a user ID as context is only supported for the edit_user capability not the edit_users capability.

      Create forum topic from comment

        • Timothy is correct, the edit_users check was added before the edit_user check, both are now checked, so the plugin author was trying to use edit_user, but wasn’t clear that he also had to use edit_users to make it completely safe.

          Create forum topic from comment

        • Way to miss the point, @Trevor! We all know the developer wrote “edit_user”. That doesn’t mean that is what he meant to target.

          In fact, there are a whole series of reasons to believe he meant “edit_users”. The most blindingly obvious one is staring at everyone right at the top of Jeff’s post. It’s the screenshot of the capabilities that the plugin exposes. You see, ironically, the plugin is about making use of the very functionality that has been misapplied here.

          Tell me, please, where on that screenshot you see the function “edit_user”. (I’ll make it easy for you. You don’t.) But you do see “edit_users”.

          So why are you and Timothy Jacobs so keen to label the developer an idiot?

          Much more important is what are you proposing to ensure something similar doesn’t cause problems in future. On that you seem remarkably silent.

          As I said, way to miss the point …

          Create forum topic from comment

        • I did read it, you should read it more carefully. If you did, you’d notice that the original edit_user check is still there. This is because the edit_user check is the more specific check to make. If he simply forgot an s like you claim, why didn’t he just add an s?

          KTS915, the reason you won’t see edit_user in the screenshot is because the two are entirely disconnected. The usage of edit_user(s) in the security fix is being used as a permissions check to make sure the current user can edit the user before assigning them their new capabilities.

          The reason you don’t see edit_user in the screenshot as a capability is because it is what we refer to as a meta-capability. These aren’t assigned directly to users, but are evaluated in a certain context like whether a given user can edit_user a certain user ID, or delete_post a certain post ID.

          If you want to see for yourself, you can take a look at the map_meta_cap function in the code reference. As you can see, capabilities are very complicated in WordPress. What is important is that the developer released a security fix.

          Create forum topic from comment

        • I guess my mistake was commenting on this in the first place, not sure what I expected after reading all the other responses.

          I never called anyone an idiot, and not that Tim needs my help, but he is correct.

          Create forum topic from comment

        • @Trevor and Timothy Jacobs,

          A word to the wise: when in a hole, stop digging.

          You mentioned the word evidence. That has legal connotations. And this is not the playground: the burden isn’t on the one who gets asked first. The burden in law is on the one alleging the higher level of fault.

          Ron and I said the developer made an innocent, non-negligent, everyday sort of mistake: a spelling mistake. You two, on the other hand, have alleged that the developer of a plugin about capabilities didn’t know what he was doing with a particular capability! Are you asking to be sued for libel, or do you just not think through the implications of what you write?

          But that’s all a distraction. I’m still waiting for your proposals on how to avoid a similar occurrence in future … You remain strangely silent on that.

          Create forum topic from comment

        • I understand that you think leaving off the ‘s’ is less of a mistake, but in actuality, at least in this situation it’s not. If you look at the code in the WordFence article linked above you’d see the mistake he made. It’s a very easy mistake to make, and important to underline here, so others don’t make the same mistake. I know I’ll be more careful with this in the future.

          Create forum topic from comment

        • GPL software is offered with no warranty.

          What you don’t seem to understand is that it is not a spelling mistake. Writing a static analysis tool to look at a piece of code and determine if it is secure or not is practically impossible. This bug wouldn’t have been caught because checking current_user_can( 'edit_user', $user_id ) is 100% valid in certain contexts. The two different checks aren’t radically different.

          Many people check every single line of code, by hand, that goes into WordPress core, it has quite good test coverage, and still security vulnerabilities exist. As they do in ALL software.

          Create forum topic from comment

        • GPL software is offered with no warranty.

          More obfuscation, old son. Totally irrelevant. (You seem to have a liking for using legal terminology, but seem equally to have no idea what it means.)

          Many people check every single line of code, by hand, that goes into WordPress core, it has quite good test coverage, and still security vulnerabilities exist. As they do in ALL software.

          Yet more obfuscation! Obviously, it’s not “quite good enough”. And that’s hardly a reason not to improve the systems for checking. Actually, all you’ve done with your comments is give weight to mark k’s earlier observation that WordPress’s current quality control model is just broken.

          “The two different checks aren’t radically different.”

          My point exactly, old chum. Not well designed, is it? As I said earlier, “That’s just asking for trouble.”

          Create forum topic from comment

    • Already there @Jeff ;) To @Ron’s defense: he has many valid end-user grievances Devs should heed—though he didn’t exactly express himself in a chillaxable light. For me, I’m not a huge fan of actionable caps. Caps should be defined and bootstrapped once. I’d rather roll into a MU plugin, or then …assign crazy-checks all throughout my code. Just my preference. Lastly, I don’t agree with Mr. Ron, but I’ll hear him out. No need to go Commental yo :)

      Create forum topic from comment

      • I’ve fought this fight since OSCommerce was known as The Exchange Project (March 2000).

        There’s a HUGE disconnect between the “it’s free – get over it” dev community and the end users who need things to just work with no major headaches.

        I’ve been straddling this fence since 1994.

        Comments like:

        “Every software has bugs period.”
        “hmmm, the plugin author didn’t force you to install it.”
        “Ron, plugin in the directory is not a service. Developers just share their code there.”
        “GPL software is offered with no warranty.”
        etc, etc, etc

        WP will always be a good choice for small businesses because it’s free.

        WP will always be a good choice for large businesses because they have a team of geeks that can do whatever they want (or pay a company to do it for them).

        Where WP is missing the boat is in between. The businesses that are willing to spend some money but don’t have a budget for a team. Think 1 geek that knows his stuff (in this case that’s me :)

        Think $5mil to $50mil or so annual sales.

        This is the sweet spot IMO that a lot of you are blowing. Small don’t want to spend money – large are going with the big players (or internally) – medium are going where? To those devs that approach it from the other side of the table and not the dev side.

        Medium businesses don’t give a damn how you get there. Save us the gory details. Does it work? Can I understand it and use it in my everyday work life? Great! You get paid.

        If WP is serious about really growing in the business market the attitudes above will have to change.

        My apologies to Vlad – you did the right thing and handled things professionally. I purposely threw out flame bait to see what responses I would get. It appears not much has changed in the open-source world when it comes to devs attitudes.

        That’s a shame….

        Businesses don’t want to see drama from the dev community. Businesses don’t care that some dev spent 100s and 100s of hours creating something for free. In business we just want something that works – period.

        The first time you throw attitude at a medium sized business owner about open-source and no warranties, etc, you’ve lost them for good.

        Just some thoughts to chew on….

        Carry on….

        Create forum topic from comment

    • Simply, because he lacks the skills to do anything else but rant — since March of 2000 or whatever.

      It is always the loudest voices that possess the fewest skills and lack context. It certainly is easier than taking any personal responsibility. He claims businesses do not want drama, yet 80’s era flame-bait is his key to ‘winning.‘

      Create forum topic from comment

    • Well, I’m not in favor of Mr. Rons choice of words, but aside of that, his methods are solid. Provocation leads to reactions. Check.

      The discussion gehts much more lively. Check
      Folks are actually starting to think how to avoid this issue in future, and not embrace a copyleft mentality, thus “its your own fault”? Check.

      Personally, I dont see the holy grail in Unit tests. For lots of small plugins and also their programmers, this might be a moot point, because they don’t understand it. Or it’s too much of an effort; not implementing the testing itself, but maybe even just the setup.

      But more intensive testing, esp. with bigger, or “grown-large” pieces of software, is a requirement. Beta testers are great. Unit tests may help, too. IMHO code readability and certain coding standards do also help a lot.

      And the discussion certainly highlighted the point, that one might want to waaay more carefully choose the capability one is checking for. Nobody hinders you digging around in the WP Core code. It’s what I do most of the time these days, thanks to a) that idiotic move from codex to “developer handbook” (because its mostly PHPDoc with no help, examples etc. whatsoever) and b) crawling it myself gives me a much better feel, grip, knowledge of what is happening, eg. with get_template_part or locate_template, than just pointlessly testing = fishing around in the fog.

      So yes, Ron is a troll. But a helpful one. Else all would just so shrug their shoulders and say non-commital comments like “well, oh my .. happens. Let’s contine with business as usual”.

      cu, w0lf.

      Create forum topic from comment

You may also like

Newsletter

Subscribe Via Email

Enter your email address to subscribe to this blog and receive notifications of new posts by email.