Dangerous Bukkit Feature - Warning

Discussion in 'Bukkit Discussion' started by Mrchasez, Apr 6, 2014.

Thread Status:
Not open for further replies.
  1. Offline

    Mrchasez

    Hello,

    Just a warning to server owners. Bukkit has implemented a system that basically does this. You type: /{plugin}:{command} and it runs that command for that plugin. This over writes certain things, like they are able to run "/me" and other Bukkit / Essentials commands.

    For you server owners who disable your /plugins command. Too bad! You can type:
    /bukkit:help

    And it displays all your plugins. I am assuming so many of you disable the plugins command you want this hidden. The best part is that Bukkit gives you no way to disable this easily, like in bukkit.yml. (edit: I have been told to disable the bukkit part of the issue you can add negative permissions)
     
  2. Offline

    danjb2000

    Basically if you did /bukkit:help it would bypass you disabling it and let them see all your plugins.

    Also /{plugin}:{command} works
    so /bukkit:me [broadcast] works for groups that don't even have the permission.
     
  3. I have two questions:
    1. This applies to 1.7.6 only?
    2. Can they run any plugin commands without having the permission ? [need to be commands for which the permission is set up in the plugin.yml, not checked by the plugin]
     
  4. Offline

    Mrchasez


    No. Not any command. However it has been reported to have issues with certain software I won't mention. It also bypasses, say you block the command "sethome" in a WorldGuard Parkour region. This would bypass that. There are a million different scenarios it can be an issue.
     
  5. Offline

    danjb2000

    Think when you have a vanilla bukkit server, A lot of permissions are given by default, All of them can be used, as said /bukkit:me or /bukkit:help e.t.c
     
  6. Is it possible to detect plugin:cmd, say in PlayerCommandPreProcessEvent? Is it since 1.7.6 or already before?
     
  7. Offline

    Mrchasez


    Considering we aren't upgraded to 1.7.6. It's before.
     
  8. Indeed, i might've read the commit, but totally misjudged the impact.
     
  9. Offline

    Necrodoom

    All this does is allowing you to use previously overridden commands. The fact that you gave permission to people to use commands you dont want them to is the problem.
    If you dont want bukkit /help, negate bukkit.command.help.

    Also, for your worldguard problem, this means you need to block all aliases of a command, like before. This just means you have a few extra aliases to block.

    After all, this is not a dangerous feature, but just one you ignored when you configured your plugins.
     
    JaguarJo likes this.
  10. It kind of got added at some point of time (Edit: like 1.7.2?). Could've been easy to overlook. Not that dangerous of course.

    So what slightly hurts would be like:
    - The command API in Bukkit is not too well suited for generic operations, lacks ways of referencing and lookup.
    - The API does not give convenient control for removing unwanted aliases or hiding commands from tab completion. Look up and altering permissions for commands is slightly complex and not well supported.
    - Using the PlayerCommandPreprocessEvent can prevent executing commands and sub-commands, but does not allow preventing tab completion, nor does it allow to conveniently look up, what command would be used (alias, plugin, ...).
    - Hoping for all plugins to always implement all needed aspects is not realistic, also simple conflicts can't be easily seen using the API (same command registered for two plugins, but which one wins out? Server owners test it, but plugins will... compare CommandExecutors !?).
     
  11. Offline

    Mrchasez


    Not true and totally misguided. People were never able to use /me because Essentials over writes Bukkit in priority. We don't give them the permission to use the essentials version, they can't use it all. However now they can completely bypass Essentials and use bukkits. Sure we can add a negative permission node, but what if this was something more serious then a me bypass? The damage would have already happened. There are also plenty of other negative issues it brings.

    Bukkit doesn't even warn anyone.
     
  12. This is a somewhat breaking change, because players now can access commands that they could not before, breaking any command protection or selection functionality even if it used overriding the command permissions, also command expansion or redirection in pre-processing events does not work anymore or can be bypassed for "old" plugins
    Concerning communication:
    I think the Bukkit-page announcements target server owners mostly, though this change concerns both server owners and plugin developers. I think EvilSeph wanted to bring more info to developers somehow (evilseph.com).

    Problem is with such changes, one needs an expert developer checking all Bukkit+CraftBukkit comits one by one, in order to be more safe :p - i don't know if there is a way to improve things, but i assume they won't experiment with tagging changes or commits too soon. Idea could be to make them searchable by tags, also adding an overview possibility what tags appear since version x-y-z. Such could allow to quickly see if the command system is affected, if core configuration of Bukkit changes, security fixes, just "behavioral" fixes. Of course it can be hard to judge if things really change any cross-plugin stuff, but this is not just an addition of a new feature possible to use for plugins, because it allows players to use it. So if we had a tag for "players can do something new", developers would find it quickly. Of course those tags would have to be flexible in that they can be added/update later on (not only in the commit message on GitHub). I am brainstorming again, things get complicated with ai changes that affect plugins, but certainly there can be practical considerations about what is enough of a change to label it - this actually could be useful to judge the impact of changes better, also for less experienced developers or server owners, or just for people with not as much time to inspect every change.

    (Edited quite a bit).
     
  13. Offline

    Necrodoom

    How is my response not true, then? You still need to negate the permission for bukkit /me to block it.
    All default given bukkit commands cannot cause any dangerous effects to your server, so im not sure what you are at.
    Since you brought up the point of seeing plugin list, you already had to negate bukkit.command.plugins and bukkit.command.help because neither are blocked by overriding plugins usually.

    The only matter left is plugins checking commands by aliases, which is a sort of can agree, but a lot of plugins, as well as vanilla, give plenty of aliases to their commands, so i dont see whats new here.
     
  14. I finished editing the above comment. In short:
    - It's been a breaking change (old plugins don't work anymore).
    - API and configuration for commands could be better (blacklist/whitelist aliases, handle commands and all its aliases with one strike).
     
  15. Offline

    Necrodoom

    Which old plugins dont work exactly? This change did not break the current command system, nor the permission system.
     
  16. It's not strictly breaking, but i won't ...err... edit my last comment to reduce confusion.

    It breaks a range of coding techniques that have been valid before, two quick examples:
    - Getting Command instances for plugins and setting a permission for the command if none is present, or altering it to something else in order to remove it from tab completion.
    - Plugins that process the preprocess event for commands or plugins, i.e. checking for the typed label, will now not catch the fish anymore. Not only protection, but also expanding commands, redirecting aliases, generic command cooldowns.

    Of course the plugins can be updated, but i'd wish we had a system to indicate such changes, so one does not fail to notice too easily. Note that this is not only cases which could be handled with setting negative permissions.
     
  17. Offline

    JaguarJo

    This information has been available information since it was posted in February. http://forums.bukkit.org/threads/craftbukkit-1-7-2-r0-3-is-now-available.232215/ Specifically:
     
  18. Offline

    TnT

    Every Bukkit command has a permission node. Remove the node if you do not want to allow its use.

    Every command has a permission node. Remove bukkit.command.help if you don't want to allow its use.

    You don't add negative permissions, you negate the permission. Just like every Minecraft Vanilla command has the same ability.

    Educate yourself. See below:
    http://wiki.bukkit.org/Commands.yml
    http://wiki.bukkit.org/CraftBukkit_commands
     
    drtshock likes this.
  19. Now that's appropriate. I think i read this one, but seems i did not get it. One might still add tags to such announcements and filters to search :p.

    Thinking of permission attachments from multiple plugins and the inability to reliably negate a permission of the attachment of another plugin without accessing the other attachment, i might still be slightly confused on how to call it, if a plugin tries to negate a permission (informal)... it can just end up adding a negated permission, without it having any effect.

    Too simple to find!

    The commands.yml allows a lot of control, so those points are somewhat invalid now (except maybe the lack of accessibility from plugin side, also generic operations to "fix" command permissions and altering the messages is slightly painful).
     
  20. Offline

    TnT

    We can't fix permission plugins. If one permission plugin is confusing to use, try another. There are many to choose from.
     
  21. Offline

    Mrchasez


    The issue is for things out of bukkit. I edited my post before you posted this to say you can fix the bukkit side by "negating" permissions. Yes, I realize you negate but it's pretty much the same thing in this context.

    People were pointing out all the issues this can cause else where, and everyone I spoke too apparently missed this update or forgot about it. So creating a thread about it seemed pretty logical.

    You can close it though. I see no further point in keeping it open.
     
  22. Offline

    TnT

    If you mean plugin commands, that requires plugin permissions. I don't actually see any issue at all.
     
  23. Not so important, i was not having permission plugins in mind, instead rather:
    - Plugins that don't set a command permission (such exists and will continue to exist).
    - Altering the permission messages of individual commands (e.g. hiding presence of commands to normal players).
    - Altering permission defaults. (Allows to keep related config in one place, though permission plugins could also have such a feature.)
     
  24. Offline

    Amaranth

    This has always been something you had to worry about. If you had two plugins that registered /help one would get /help and the other would get /<pluginname>:help. We've just made the /<pluginname>:help version always get registered for consistency.
     
  25. Offline

    Bobcat00

    So, block:
    1. /sethome
    2. /esethome
    3. /essentials:sethome
    That's what I do. Problem solved.
     
  26. Offline

    Necrodoom

    For that point, instead of grabbing a whole new plugin to override its command, which may or not may work, youd set a permission in its plugin.yml, or remove the command completely. If it doesnt register it in the first place, well, you couldnt override it anyway, so youd need to head to commands.yml and override all its aliases anyway.
     
  27. Consider it implemented in a plugin that is on the server anyway, while the plugin that has a command without a permission set is from another author. Editing jars all the time with updating is possible and might even be automatized (*oops*), but that's not more simple than running an extra plugin - i understand the attitude though :).
     
  28. Offline

    Mrchasez

    I dont have an issue. It's an example. Say you have a thousand commands, you are going to manually do that to them all? The thread is to make a point. No longer need to continue it.
     
  29. Offline

    Necrodoom

    If you had to do it manually before, youd need to do it manually now.
    I dont see the difference.
    This is in no way a 'dangerous' bukkit feature.
     
  30. Seen on another level of abstraction it was a breaking change. I'd also not hope to prevent real danger to the server by overriding aliases, instead of using permissions, however it could break safety-nets built on base of the command-preprocess event, if relying on message parsing only (...) - one could somehow get the SimpleCommandMap instance to fetch the Command that should be executed and use the label for comparison then, which seems not to be touched by the change.

    The change was announced about the same time of the beta release, so i don't think there is much to debate, except maybe having thousands of servers running legacy plugins :p, and that one better should read the beta announcements :p.
     
Thread Status:
Not open for further replies.

Share This Page