1. Guest, as per the stickied thread, this forum has not been in use since 2014. All bugs and feature requests should be posted to JIRA.

Solved [Security] Force close inventory's on impossible player actions

Discussion in 'Bugs & Feature Requests' started by MyPictures, Jul 15, 2013.

  1. What is the problem?
    Currently (because Mojang is smart) you can walk, run, fight and do other things while having your inventory open and manageable. This gives other players without any hack tools a big disadvantage compared to the hackers.

    What should Spigot do instead?
    Close the players inventory if he/she is trying to:
    1. Hit an entity (careful THORNS enchamtnet!)
    2. Interact with something
    3. Break someting
    4. Place something
    5. Chat something
    6. Walk/run somewhere (Player velocity and falling might get annoying here...)
    7. Eat something (not sure if really possible)
    8. Fire an arrow with a bow (not sure if really possible)
    9. Maybe something else?....
    While having a inventory open (E, chest, hopper and other).

    Spigot should also prevent this even if a player is ignoring that his/her inventory got force closed (freecam).

    Does this give any bonuses?
    Yea this would limit the "AutoSoup" hack at a good amount since players cant move items around any longer while fighting or running.

    Any other information?
    NoCheatPlus and AntiCheat aren't really covering this at the moment. All they do is prevent too fast inventory management which doesn't really catch it at all.

    Original idea by asofold and extended by me.
     
    #1 MyPictures, Jul 15, 2013
    Last edited: Jul 16, 2013
    • Agree Agree x 8
    • Disagree Disagree x 1
  2. Sounds reasonable, Think you can make a PR?
     
  3. joehot200

    Supporter

    Obviously would provide more lag than an in-built thing, but why not just make a plugin that monitors all these events and closes the players inventory if it is open?
     
  4. If that approach was taken to all suggestions, then there would be no need for suggestions at all...

    The point being, its more efficient to do in the server itself and provide it out of the box instead of users having to install plugins for it.
     
    • Agree Agree x 3
    • Like Like x 1
  5. Implementation suggestion to keep minimal diff:

    in generic incoming packet handler, do

    Code (Text):
    Spigot.checkForActionPacket(player, packet); // Spigot
    then checkForActionPacket would do
    Code (Text):
    if (player.activeContainer != defaultContainer) {
        if (packet instanceof Packet1 || packet instanceof Packet2 || packet instanceof Packet3) {
          player.closeInventory();
      }
    }
     
    • Like Like x 1
    • Agree Agree x 1
  6. I'm not experienced enough with either Craftbukkit or Spigot so I'm not sure where the best place for this little check in Spigot would be. Also I'm quite bad experienced with Minecraft/Bukkit development, all I do is little tiny plugins for some small server changes.

    I doubt there will be any noticeable lag having this enabled but someone could also provide a little setting in the spigot.yml to turn it on/off. You probable lose more performance if you let your players fight/move/chat while managing inventory.

    This is true. I'm having a look at all server weakness points that I and asofold have found over the month and try to provide some little tiny check ideas that would work perfectly without any false positives. Spigot already comes with some nice additions such as dupe fixes, selfhit check (Mojang fixed this also now in 1.6.2) and some other little things.
     
  7. SuperSpyTX

    Supporter

    #7 SuperSpyTX, Jul 15, 2013
    Last edited: Jul 17, 2013
    • Agree Agree x 1
  8. Nice to hear that.
     
  9. Added some little (comments) that might be useful. Thanks to asofold .
     
  10. Would be nice if it threw an event to, say PlayerImprobableEvent, so someone can make their own decision to blow the player up.
     
    • Like Like x 3
  11. I'm pretty sure there's already a server-side implementation of this in mc/bukkit/spigot, honestly why wouldn't you be checking if the players inventory is open when they try to manipulate it.
     
  12. Sorry for the really late response but I somehow missed it: No there actually isn't any protection against this at all. CraftBukkit has inventory open and close events but nothing more.
     
  13. Completely disagree. This could add lag and cause problems with authentication plugins, magical plugins, and stuff like this. This is more sounds like request for nocheatplus and can be 100% completely done with plugins.
    Don't forget that inventory close packets also closing chat!
     
  14. joehot200

    Supporter

    I agree this can be easily bypassed by opening the players inventory, moving the item to the place you want, and then closing it again in 0.00001 seconds.
     
  15. I think your confusing the issue with a different scenario.

    This request is to prevent stuff like moving and chatting while inventory is open - stuff that typically isn't possible.

    It would also be simple to gate this behind an anti-cheat flag.
    Not sure you understand this either - Has nothing to do with authentication, and plugins shouldn't be relying on cheat client features to work...

    Regardless, it would be behind an on/off switch.

    It also would not cause any lag.
     
    • Agree Agree x 1
  16. You probably will get more lag if you allow them managing their inventory while running around or doing other stuff together.
    Why would authentication plugins be a problem? You wont get false alerts if you for example run /give on you or if another plugins decides to remove something from your inventory.
    Yes you could do this with a plugin since the events do in fact exist but Spigot also has Anti-Xray (can be done by plugin), log filter (can be done by plugin) and other stuff also. So I thought this would be a nice little addition since it wont really cause any false positive (depends on what scenarios you want to cover).

    Thats not actually bypassing more aimboting. We could also set a limit for opening and closing the inventory to something more human like but the current inventory handling is ridiculous a player shouldn't be allowed to auto manage his/her inventory all the time without even having to open it.
     
    • Agree Agree x 1
  17. md_5

    Administrator Developer

    Implemented 1-4.
     
    • Like Like x 1
  18. Thread necromancy...... :p
     
  19. MD5 IM GOING TO BAN YOU FOR THAT NECRO
    ;)
     
  20. Build #1601, your commit, prevents opening of all containers. Make a revert quick! ;)