search

samolego / GolfIV

An anti-exploit attempt for Fabric
https://modrinth.com/mod/golfiv
MIT License
49 stars 13 forks source link

Potential overflow issue in ServerPlayNetworkHandlerMixin_TimerCheck.java #26

Closed jukefr closed 3 years ago

jukefr commented 3 years ago

I've been messing around with your project on my anarchy server. I have quite lax needs, just need an antifly and antielytra to save the tps, anything speed related is fine as long as its not too out of hands.

So I was looking around the src/main/java/org/samo_lego/golfiv/mixin_checks/C2SPacket/ServerPlayNetworkHandlerMixin_TimerCheck.java file and logging the packetRate value and I think there is a problem with the logic.

You expect a default packet rate of 50 and if everything is going well and the client timer is around 1 the packetrate will hover around 0 overall during cycles.

If you go over 1 then the packetrate will eventually go over 250 and an offense will be logged.

However you didn't account for the case where a client would set a timer to a value less than 1, and I think it would eventually overflow (packetRate will just keep growing in the negative) if a user managed to stay moving long enough with it for the counter to reach that point, and even if it doesnt overflow it just takes up extra memory for no real use.

I know it's a bit far fetched but I think it shows a problem with the current check (in my view at least). I'm making changes on my side anyways to allow people to use a timer up to x2 so that 50 number will have to go, I might make a PR if you are interested.

samolego commented 3 years ago

Humm, don't you think it might be simpler to check if rate is less than e.g. -1000 and then reset it to 0 / kick the player?

Using timer x2 is actually quite a lot imho ...