Added a config option to enable/disable sending transfer packets

[Biquaternions]

Transferring a player will require both sender and receiver to have transfers enabled in paper-global.yml and server.properties respectively.

This increases security as server staff with elevated permissions can send players to self-hosted servers to ip-grab them without having console access in the "sender" server.
This also breaks attack vectors where server staff is tricked into transferring players directly via command or clicking a sign.

[lynxplay]

I am not a large fan of this change.
Paper already assigns permission nodes to all vanilla commands.
Servers concerned with this should just be removing the permission for the transfer command minecraft.command.transfer from everyone.

Has the same effect without the need for a new server option.

[Biquaternions]

I thought of something similar while reviewing the code so I made the implementation on the send packet function to also prevent plugins from doing this using packets.
So... it is not limited to just the /transfer command.

[lynxplay]

I am confused, why would a plugin transfer players if the server owner is not interested in transfers?

[lynxplay]

To follow up on this, the transfer command is not something normal players have access to.

A server would have to effectively grant operator permissions to someone for them to be able to use it or explicitly the transfer permissions. That is already incredibly dangerous, transferring seems like the least of your worries if someone has such level of permissions.

The angle of a plugin transferring has two parts.
Either the plugin maliciously does it, at which point, you have malware.
Or the plugin exposes a transfer mechanism and somehow a staff member not meant to have that permission gained that? Which is pretty much the same point as above.

I just really don't see an angle where a server might have accidental transfers enabled without it being aggressively missconfigured, which I doubt those servers will use a config option to fix that.

I'll throw it at some other members for their input, but from my point of view, I don't think I want to pull this change.

//edit: it is still a well crafted PR, and thank you for opening it!, but yea. I don't think it is a good fit for paper.

[Biquaternions]

Sometimes people upload malicious plugins to official platforms like it happened in modrinth a couple of months ago.
Other times server owners get custom plugins from other people like "friends" or worse, players.

Tbh, that last one is mostly their fault 😅
But still is something that sometimes happens, specially for non experienced server owners, and the ones at risk ends up being the players

[Biquaternions]

No problems, you can close it.
Thx for the compliment!

[lynxplay]

Yea malware certainly exist 😅 Issue with that is usually "ah fuck, malware might as well just change the parsed config value before sending this". Fighting malware outside of "do not install it in the first place" is sadly usually a lost cause.

I'll throw the PR at someone else before I close it just to make sure the team agrees with me on this. But yea, thank you already for the PR no matter the outcome!

[masmc05]

Sometimes people upload malicious plugins to official platforms like it happened in modrinth a couple of months ago.

Tbh I can't see a reason why would someone transfer people instead of silently grab the exact same information on your server and send it back to the bad dev. People will quickly start to notice those transfers and complain, exposing the plugin, all this for no extra information that a transferred server could extract.

[Biquaternions]

Sometimes people upload malicious plugins to official platforms like it happened in modrinth a couple of months ago.

Tbh I can't see a reason why would someone transfer people instead of silently grab the exact same information on your server and send it back to the bad dev. People will quickly start to notice those transfers and complain, exposing the plugin, all this for no extra information that a transferred server could extract.

True, if they managed to get the plugin on the server they can already have access to player information there.
The original idea was based on the /transfer command where the attacker doesn't necessarily have access to the console and then I thought of the plugins making the same.

But yh, limiting access to the command via permission plus the fact that plugins already have console access and can make worse stuff than just transferring players makes this config a bit pointless.

Btw, I think it would be a good idea to mention stuff like this in a dedicated section on the wiki, similar to Velocity's.
I'm sure there are more security tips that new owners will be glad to read.

[Biquaternions]

Btw, this is the first PR I make to Paper, so I'm not yet aware of your workflow if usually you close the PRs or I should be closing it.

[ShaneBeee]

I vote no on this:

1. Regarding the command, as already stated, commands have permissions and should be negated. No one but the server owner should be op, therefor no one but the server owner should have permission for that command.

2. Regarding malicious plugins.... no matter what, a malicious plugin would still be able to change the config option and force transfer a player. The config option wont stop anyone.

[electronicboy]

This implementation is far too trivial to bypass by anybody remotely familiar with the networking library in use, and wouldn't take much rocket science to see how to bypass the method here. The only way you'd be able to do this is to handle this in the pipeline itself, but, this generally just offers a fake security assurance which nobody can promise when you're running arbitrary code on your server.