Dinnerbone / mcstatus

A Python class for checking the status of an enabled Minecraft server
http://dinnerbone.com/minecraft/tools/status/
1.11k stars 146 forks source link

Negative varints are not handled correctly. #141

Closed CoolCat467 closed 3 years ago

CoolCat467 commented 3 years ago

Negative varints are not handled correctly because in Python, numbers are represented with infinite bits instead of 32 or 64, meaning right shift including sign bits does not exist, meaning this implementation only does regular right shift. This causes varints (at the moment) to be able to handle positive integers from 0 to 2 35 - 1 instead of the true range minecraft uses of integers from -2 31 to 2 ** 31 - 1. I predict no one has noticed this before because in this script's usual function, the varint implementation is only used for reading text length, and since text length is never negative it has never been noticed.

CoolCat467 commented 3 years ago

So, interesting story behind me finding this. I'm planning to use the connection module in one of my upcoming projects, and was curious about this varint thing I kept seeing in parts handling buffers and text. I saw by it's read write functions it's limited in size to 5 bytes, so I was wondering what the size constraints were and started doing research. I found a webpage about it, it told me the limits, and I decided to test it, and I found negative numbers broke it and that positive numbers could go far higher than 2 ** 31 - 1, so I started looking into why. All the code is nearly exactly the same as a C demonstration of it, except that pesky "sign bit is shifted with the rest of the number rather than being left alone" operator ">>>=" being ">>=" since it doesn't exist in python.