Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Packet cooldowns and content validation #4782

Open
wants to merge 27 commits into
base: master
Choose a base branch
from

Conversation

OurLobanov
Copy link

No description provided.

OurLobanov and others added 12 commits June 17, 2024 11:20
I do not know why they saved it if it is not used anywhere If they decide it is necessary, only one UUID is sent when changing emotions, with a limit of 50 I think that is sufficient, and the outdated ones can simply be deleted
Fix from qrvprox
check if the code is working correctly
Вроде все работает нет времени в данный момент
Comment on lines 43 to 48
if (command.length() > 512) {
// A legitimate player cannot send more than 512 characters
// This is necessary so that the conversion to plain text is not clogged
session.sendMessage(GeyserLocale.getPlayerLocaleString("geyser.chat.too_long", session.locale(), command.length()));
return;
}
Copy link
Member

@onebeastchris onebeastchris Jun 21, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same as above.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, if the game does not allow you to submit more than 512 characters from a player, I think here you can already get into the protocol

@Kas-tle Kas-tle self-assigned this Jun 21, 2024
@OurLobanov
Copy link
Author

I hope this limit will make in CloudburstMC/Protocol, I have no desire to go there
PS: A player cannot send more than 512

@@ -136,6 +105,87 @@ public void deserialize(ByteBuf buffer, BedrockCodecHelper helper, InventorySlot
}
};


/**
* The player can cause a packet error themselves, which hackers can exploit to spam legitimate errors
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't understand this comment. We now disconnect on the first error for any packet. Regardless, this seems like it should be a change in protocol, not Geyser. Changes in here should only be things we can do as a result of fields we do not need in Geyser.

import java.util.Map;

public class PacketCooldownManager {
private final Map<String, CooldownSettings> packetCooldownSettings = new HashMap<>();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use fastutil Object2ObjectMap.

@OurLobanov
Copy link
Author

The pull request is ready, please check

@onebeastchris
Copy link
Member

Unfortunately, it's not. Please address the notes in kastle's review, such as the imports. Further, some of Kastle's questions haven't been answered.

@OurLobanov
Copy link
Author

please check

@Konicai Konicai changed the title packet limit Packet cooldowns and content validation Aug 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants