kangarko / ChatControl-Red

Issue tracker and documentation for the next generation ChatControl Red, the most advanced chat management plugin.
43 stars 22 forks source link

1.20.1: Chat Packet Issue #2664

Closed gre3x closed 2 months ago

gre3x commented 2 months ago

"/version ChatControlRed" - plugin version

10.25.9

Are you using MySQL?

Yes

Are you using BungeeCord?

Yes

Error log (if applicable)

N/A

"/chc debug" output (strongly recommended)

debug.zip

Information about the issue/bug

Players are able to spoof any message on the Bungee sub-server that they are currently on through custom clients by sending plugin message packets with the correct arguments and the server name is the same as the current server.

This is related to Bungee command forwarding, but the Bungee server is not involved I think. This all happens on the spigot server. However, I think this issue is also abusible on the Bungee server.

I do not have access to these clients or the exact code of how they are doing it, since these are being used my malicious players on my server and not by me, but here is what I think is close to an example of what a dangerous packet sent on an example "Hub1" server would look like:

There is no way to block these right now without disabling Bungee integration entirely.

Can you please add an OPTIONAL configurable secret key to bungee.yml (same on every server) that is used to encrypt the Bungee packets that is needs to match with all servers on the network?

So whenever either the sub-server or the Bungee gets a Bungee packet, then it will encrypt the packet using the secret key before sending, and decrypt it upon receiving?

The packet needs to be encrypted, because if it was just a key, then attackers could intercept the plugin message and get the secret key from that interception.

The config would be something like:

# Enable secret keys processing for Bungeecord packets to prevent processing spoofed packets.
# This value MUST be the same value on every server for bungee features to work.
#
# At this point we are investigating a potential vulnerability so we recommend enabling this for
# large public servers with untrusted clients.
Enable_Secret_Key: false

# Set the secret key if you have Enable_Secret_Key set to true. It is recommended to make this string random, long, and containing special characters. You can use this website to help generate a secure key: https://www.lastpass.com/features/password-generator#generatorTool
# This value MUST be the same value on every server for bungee features to work.
#
# At this point we are investigating a potential vulnerability so we recommend enabling this for
# large public servers with untrusted clients.
Secret_Key: 'REPLACE_THIS_WITH_YOUR_SECRET_KEY'

Then the code would be something like the below: (Using encryptWithAES_CTR() and decryptWithAES_CTR() code from: https://stackoverflow.com/questions/30511903/what-is-the-simplest-way-to-encrypt-decrypt-a-byte-array-using-bouncycastles-b)

On plugin loading:

onReloadablesStart() {
      if (config.Enable_Secret_Key) {
          byteArray = encryptWithAES_CTR(byteArray);
      }
      this.bungeePacketKey = new KeyParameter(config.Secret_Key);
}

public KeyParameter getBungeePacketKey() {
      return this.bungeePacketKey;
}

Sending plugin message:

public static <T> void sendPluginMessage(@Nullable Player sender, String channel, BungeeMessageType action, T... data) {
      if (config.Enable_Secret_Key) {
          byteArray = encryptWithAES_CTR(plugin.getBungeePacketKey(), byteArray);
      }
      sender.sendPluginMessage(plugin.getInstance(), channel, byteArray);
}

Receiving a plugin message:

public void onPluginMessageReceived(String channelName, Player player, byte[] data) {
    if (config.Enable_Secret_Key) {
         data = decryptWithAES_CTR(plugin.getBungeePacketKey(), data);
    }
    ByteArrayInputStream stream = new ByteArrayInputStream(data);
}
gre3x commented 2 months ago

FYI, the paper docs on plugin messaging specifically call out this spoofing possibility as a warning when using plugin messaging: https://docs.papermc.io/velocity/dev/plugin-messaging#:~:text=There%20are%20two%20methods%20to,what%20you%20need%20to%20achieve.&text=On%20your%20backend%20server%2C%20only,your%20proxy%2C%20and%20spoof%20them.

You can also see that it is discussed here and determined that the only truly safe approach is to encrypt the data: https://www.spigotmc.org/threads/can-plugin-messaging-channel-be-hacked.387330/

gre3x commented 2 months ago

@kangarko I believe this should be a high priority

kangarko commented 2 months ago

Thanks. I am busy fixing paper breaking our plugins on 1.20.5 but I will have a look with high priority in a few days

kangarko commented 2 months ago

By the way, thank you so much for providing a great bug report. This is one of the best tickets I've seen in a long time.

If you have a chance to add your suggested changes, I've invited you to ChatControl's source code.

All you need to compile is ChatControl and Foundation. Here is the logic for sending messages to bungee, and here is for reading them.

Bungee/Velocity parts are found here and here and their libraries are on my github with the same packaging/structure/logic.

kangarko commented 2 months ago

I use the channel name plugin:chcred, not sure if the problem would go away if I just rename it to BungeeCord and then store the channel name in the byte array as the first utf?

kangarko commented 2 months ago

I have reworked the protocol to work under BungeeCord channel since that one appears not to be spoofable by clients. Use the next releases out by tonight.

gre3x commented 1 month ago

Awesome thank you so much! Sorry I've been extremely busy so I hadn't gotten a chance to respond, but I just updated to the latest version and everything seems to be working well! I'll be sure to make PullRequests if I encounter any further issues :)