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

Variable package size limit #3

Open
Krzmbrzl opened this issue Oct 2, 2022 · 4 comments
Open

Variable package size limit #3

Krzmbrzl opened this issue Oct 2, 2022 · 4 comments
Assignees

Comments

@Krzmbrzl
Copy link
Member

Krzmbrzl commented Oct 2, 2022

Instead of using a hard-coded max size for receiving UDP packets limits what kind of extensions can be made in the future as older clients would simply drop incoming packets that they consider too large, even if technically the Mumble protocol now allows larger packets.

Thus, it would be better to simply take packets in the size in which they come, with some sane upper bound to prevent malicious packets from overflowing a peer.

@davidebeatrici davidebeatrici self-assigned this Oct 2, 2022
@davidebeatrici
Copy link
Member

d5215ef

Maybe we should increase the default size limit to 4096 or so.

@Krzmbrzl
Copy link
Member Author

Krzmbrzl commented Dec 8, 2022

d5215ef

Maybe we should increase the default size limit to 4096 or so.

That would still be a hard-limit. The idea is to essentially accept any package size length and resize the buffer dynamically as needed. And in order to not be vulnerable to malicious packages, we should have a hard upper bound, but that should be at least an order of magnitude larger than the largest expected package (maybe around 500KiB or so)

@davidebeatrici
Copy link
Member

In that case startUDP()'s bufferSize argument can be (renamed and) used to specify the upper bound, with a default of 500000 bytes.

We can keep the initial buffer size at 1024 bytes.

@Krzmbrzl
Copy link
Member Author

Krzmbrzl commented Dec 9, 2022

Yeah, sounds good 👍

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

No branches or pull requests

2 participants