EssentialsX

EssentialsX

2M Downloads

BungeeCord issues

TheMuffinPony opened this issue · 10 comments

commented

Hello, all. I’ve noticed this bug on a server (where I don’t have access to any Essentials information, i.e. server logs/config/version, etc.) with BungeeCord, although this bug probably wouldn’t benefit from any of those.

Details

When going to another server (connected with BungeeCord), player data is separated, i.e. through /seen. This means that a player can be on another subserver, but is offline according to /seen.
This might not seem to be too big of a problem, although some other commands are glitched (that shouldn’t be). For instance, going to subserver no. 1 and doing /heal heals you, however (for example), a one minute cooldown is in place. Going to another subserver and doing /heal again will also heal you, thus allowing you to partly bypass the heal cooldown.

Reproduction

Go to a server with BungeeCord and EssentialsX, and do /heal twice. The second time, you should not be able to do /heal because of a one minute cooldown (if this doesn’t happen, ensure you don’t have the permission essentials.heal.cooldown.bypass and essentials.feed.cooldown.bypass, and ensure that a cooldown is set in the config). Now, go to another subserver, and do /heal. You should heal successfully, despite the fact that the cooldown in the first subserver is still ticking down (and thus, should not heal).

If fixed

Add some way to group this or add some hook into a plugin that preserves inventories/health, etc. between subservers. Then, add some way to transfer this cooldown using that plugin, somehow. Also, hook into the grouping part so that bypassing cool downs doesn't work between two ‘linked’ subservers (i.e. survival world 1 and survival world 2), but does work between two unlinked subservers (i.e. a survival world and a creative world).

commented

I imagine it might be possible to do this if you just symlink the plugin dir that all the servers use to a NAS.

commented

People are welcome to develop and contribute a means to synchronise data across BungeeCord instances, but this isn't currently something we're planning to add directly to EssentialsX.

commented

I don't suppose that would be such a difficult fix to save player data when they leave would it?

commented

It's not that, it's that the player might start to join the next server before EssentialsX had finished cleaning up and saving the data on the previous server. As far as I know, EssentialsX saves userdata as soon as the user disconnects, but this might not be quick enough.

commented

@AgentTroll Someone was doing that in a previous ticket, but I don't think EssentialsX saves on the previous server before the player joins the next server, meaning the userdata is stale after switching servers.

commented

This may be able to be solved using file locks, however they would be incredibly prone to user error. There is a definite race condition when using Bungeecord to change servers - Bungeecord will leave the player 'joined' to the old server and will only close the connection to the old server once the player has received a join server packet from the new server, long (in computer time) after Essentials has loaded the profile on the new server.

commented

Essentials delays loading of user data iirc, the player will get the chance to receive the packet

commented

@Ichbinjoe This approach also means that we would need to wait for the lock to be released before we load the data, and this could cause a noticeable delay in the user being able to use EssentialsX.

An alternative is to watch and reload user data, but I don't know what side effects that would cause.

commented

@ Ichbinjoe This approach also means that we would need to wait for the lock to be released before we load the data, and this could cause a noticeable delay in the user being able to use EssentialsX.

Wouldn't this be what we want? I mean obviously we wouldn't want a 'noticeable delay', however I can't see a good way of ensuring a 'double dip' of something like a heal without some sort of locking mechanic. Granted, we can probably mitigate a lot of the delay before the loaded data by constantly saving and only gaining a lock on the file when we need to edit it (example being a /heal with a cooldown).

Imagine the following attack scenerio:

  • Log onto one server on the network. Perform /heal
  • Immediately do /server (lets pretend this is bungeecord land)
  • Immediately perform another /heal, as the last server has not yet 'saved' our data, or we have not yet reloaded the player's data

This sort of rapid succession can be achieved with a modified client / bot designed to perform chat commands quickly, and can be exaggerated by leaving a low TPS server and joining a high TPS server (giving the attacker more time before the low TPS server fires a PlayerQuitEvent).

While I do believe this is more of an advanced attack (and theoretical at that, I would have to look up more about timings and how Bungeecord + Spigot + Essentials would interact with eachother timing wise), I think its worthwhile to mitigate because players on more competative servers could see this as an attack vector (whether purposely finding this issue, or whether stumbling on it by mistake).

I am not convinced that locking a file is a good solution, however I think that whatever solution is adopted (if any) should include some sort of mechanic to ensure both atomicity as well as consistency across the network which this feature is installed on. When attempting to enforce rules, eventual consistency is often inadequite and can lead to these types of corner cases.

commented

Closing in favor of #912 which suggests adding a BungeeCord module (which in theory would be used to add support for BungeeCord). Currently Essentials does not support data syncing across BungeeCord, or using MySQL.