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

[Cheat Detection] Anti-Cheat reimplementation #1434

Merged
merged 23 commits into from
Aug 31, 2021
Merged

[Cheat Detection] Anti-Cheat reimplementation #1434

merged 23 commits into from
Aug 31, 2021

Conversation

dencelle
Copy link
Contributor

No description provided.

@dencelle
Copy link
Contributor Author

okay, this doesn't seem to be getting taken down so, i'll actually write credits to all the people who deserve credits for this project happening.
@Akkadius - guidance on proper formatting and helping a ton with getting a few bugs worked out
@mackal - all the help with MovementHistory
@SecretsOTheP - help with countless amounts of anti-cheat checks and tips on things i didn't know the client sent
ailia - helped a lot with the math to make sure the deletaT was as close as possible without false triggering
@RoT-PvP - helped with testing the system on their server and has been a huge help with giving feedback
@hgtw - guidance on proper formating
@Kinglykrab - providing the code for implementing into quests

@splose

This comment was marked as abuse.

@ghost
Copy link

ghost commented Jun 20, 2021

I have a pull request open that features the full functionality. Can easily copy paste over the changes and stuff here into the fully functional version.

#1433

@ghost
Copy link

ghost commented Jun 27, 2021

I have been using this PR for 5 days. Also during a load test of my server. No issues. at all.

@splose

This comment was marked as abuse.

@dencelle
Copy link
Contributor Author

i just wanted to clarify on where this whole "movement manager" argument came from. ailia and secrets tried to put in a* tracking on classless, it failed because its too cpu costly. I've made several attempts at doing it myself but the only result i have ever had end in no crashing and no false positives resulted in my computer using up 90% of its cpu power for just 1 client online (i have a 6 core 4.5ghz cpu running my server). in my personal opinion an a* tracking of players would in-fact be seriously beneficial to eqemu but i don't see it being a realistic thing. its been a theory for a long time that someone could do it but in all reality i do not see this being done any time soon and would probably require a rework of a lot of eqemu's code. with all that said, movement manger still has pitfalls and would still get a lot of false positives since the client itself has hard coded zone lines that eqemu's movement manger does not account for.

@dencelle
Copy link
Contributor Author

And I saw that you were creating exceptions for certain zones is that due to the zonelines?

these exemptions are for the zonelines that are hard coded in all eqemu clients. in the past VZTZ bypassed this by simply making an exemption for those said zones. this left a large vulnerability in the anti-cheat code since those zones were pretty much labeled as a free-to-warp zone (bothunder being the most popular one people would freely warp since the whole zone was exempt). this was removed since we implemented MovementHistory that took over the port exemption of these exemption areas in a fashion that was better and more secure in the long term.

How come you didn't just mention that before? I would've been willing to work with you on optimizing it. I would guess its the formula for distance that is bogging everything down?

i was unaware (honestly have never seen) secret's and ailia's version of this movement manager tracking, the issue that i was facing with it didn't have anything to do with the distance (we already do a distance check via TZVZ's implementation and that is what triggers the Large Warp Detection method. movement manager would be an a* comparision to see if each movement would be valid from the last location we were at to the current location we are at. sadly, not every packet is sent to the server since EQ used UDP and this resulted in some dropped packets (only 2/10 movement packets actually ever make it to most servers according to sequence numbering) we could do a movement check via MovementHistory, that is what i tried at first, but i came to the quick realization that this could be falsified easily and would yield terrible results. (along with processing ~300 movement updates per client that's moving per 0.3-1.5 seconds is very costly) its not the math that is really the issue, eqemu already has that in place, its just the amount of calls per a packet that would be the issue, even on a fairly decent computer 1 client can clog that up. navmesh would also need to be updated for each zone to a higher definition since i was getting a lot of false positives (tutorialb through the cave to the spider there is a client side web that messes with this method since it sees a wall, any invisible wall as well also since navmesh does not account for these currently very well). to ease you're worries of it being a dropped subject, i will give you in-sight on what i am doing. i have been working on it and will remain working on it since as i said in my original statement i do want it to be in and it would ultimately kill any chance of warping. currently i do not see this as being a viable solution, but i do see it being viable in the future once enough work and thought has been put into it.

as for this PR. it has been tested on multiple servers, has been proven to be stable without crashes and (as of me writing this) no one has reported any significant false positives and it has helped some servers detect people using mq2 in malicious ways. that was the intent of this PR and unless someone has a valid objection as to why it shouldn't go into master branch, i think we should proceed with it being merged.

@dencelle
Copy link
Contributor Author

Well like I said i'm willing to work with you on it.

I have been saying merge this literally since your first commit. Don't really think I should be banned as I was the least involved in this entire situation but Turmoilsnake gotta prove his point.

If he actually keeps his word and unbans me (unlikely) and gives me all my access back I will see about committing what I have as an addition to your system.

this is an issue between him and you. I'd like to keep that issue off of this PR since it doesn't involve whats going on with this PR. if you have any further questions about my design choices or code that is in this PR I'll be more than happy to answer them for you and I'll make an attempt to give you as well thought of an answer as i possibly can as soon as i possibly can. but i do request you keep the personal issues between you two off this PR and let discussion relating to this PR to be strictly about any outlining issues that i could of missed (false positives, crashes or buggy code)

@fryguy503 fryguy503 requested a review from mackal June 29, 2021 12:06
@ghost
Copy link

ghost commented Jul 1, 2021

I am getting false positives with players on Sirensbane. This is from oot:

[07-01-2021 :: 14:44:24] [Zone] [Cheat] /MQWarp (large warp detection) with location from x [-5288.92] y [251.61] z [4.58]
[07-01-2021 :: 14:44:24] [Zone] [Cheat] ChangedPlayerName moving with a speed 6.11957. Expected speed: 4 Distance: 18.2328
[07-01-2021 :: 14:44:27] [Zone] [Cheat] /MQWarp (large warp detection) with location from x [-5163.52] y [250.10] z [4.56]
[07-01-2021 :: 14:44:27] [Zone] [Cheat] ChangedPlayerName moving with a speed 6.10313. Expected speed: 4 Distance: 60.569
[07-01-2021 :: 14:44:30] [Zone] [Cheat] /MQWarp (large warp detection) with location from x [-4960.65] y [236.31] z [4.51]
[07-01-2021 :: 14:44:30] [Zone] [Cheat] ChangedPlayerName moving with a speed 6.09086. Expected speed: 4 Distance: 16.9379
[07-01-2021 :: 14:44:33] [Zone] [Cheat] /MQWarp (large warp detection) with location from x [-4800.88] y [224.84] z [4.59]
[07-01-2021 :: 14:44:33] [Zone] [Cheat] ChangedPlayerName moving with a speed 6.00146. Expected speed: 4 Distance: 62.4828
[07-01-2021 :: 14:44:36] [Zone] [Cheat] /MQWarp (large warp detection) with location from x [-4596.94] y [202.10] z [4.54]
[07-01-2021 :: 14:44:36] [Zone] [Cheat] ChangedPlayerName moving with a speed 6.10218. Expected speed: 4 Distance: 17.5864
[07-01-2021 :: 14:44:39] [Zone] [Cheat] /MQWarp (large warp detection) with location from x [-4437.24] y [189.23] z [4.61]
[07-01-2021 :: 14:44:39] [Zone] [Cheat] ChangedPlayerName moving with a speed 6.03667. Expected speed: 4 Distance: 62.5097

Is there a way to either exclude a zone completely or find out if players are boarded?

@dencelle
Copy link
Contributor Author

dencelle commented Jul 1, 2021 via email

@SecretsOTheP
Copy link
Contributor

SecretsOTheP commented Jul 1, 2021 via email

@dencelle
Copy link
Contributor Author

dencelle commented Jul 1, 2021 via email

@ghost
Copy link

ghost commented Jul 2, 2021

Some False Positives. Troll Shaman in ecommons:

[07-02-2021 :: 07:40:07] [Zone] [Cheat] ChangedPlayername moving with a speed 4.08882. Expected speed: 4 Distance: 11.5832
[07-02-2021 :: 07:40:16] [Zone] [Cheat] ChangedPlayername moving with a speed 4.00347. Expected speed: 4 Distance: 39.6369
[07-02-2021 :: 07:40:24] [Zone] [Cheat] ChangedPlayername moving with a speed 4.11661. Expected speed: 4 Distance: 12.157
[07-02-2021 :: 07:40:27] [Zone] [Cheat] ChangedPlayername moving with a speed 4.02355. Expected speed: 4 Distance: 12.2089
[07-02-2021 :: 07:40:29] [Zone] [Cheat] ChangedPlayername moving with a speed 4.08005. Expected speed: 4 Distance: 11.4252
[07-02-2021 :: 07:40:32] [Zone] [Cheat] ChangedPlayername moving with a speed 4.0693. Expected speed: 4 Distance: 11.0387
[07-02-2021 :: 07:40:35] [Zone] [Cheat] ChangedPlayername moving with a speed 4.01577. Expected speed: 4 Distance: 29.9963
[07-02-2021 :: 07:40:54] [Zone] [Cheat] ChangedPlayername moving with a speed 4.09101. Expected speed: 4 Distance: 14.0534
[07-02-2021 :: 07:42:22] [Zone] [Cheat] ChangedPlayername moving with a speed 4.25641. Expected speed: 4 Distance: 11.4303
[07-02-2021 :: 07:42:35] [Zone] [Cheat] ChangedPlayername moving with a speed 4.21725. Expected speed: 4 Distance: 11.8382
[07-02-2021 :: 07:42:46] [Zone] [Cheat] ChangedPlayername moving with a speed 4.04651. Expected speed: 4 Distance: 18.0258
[07-02-2021 :: 07:43:05] [Zone] [Cheat] ChangedPlayername moving with a speed 4.04294. Expected speed: 4 Distance: 39.9543
[07-02-2021 :: 07:43:17] [Zone] [Cheat] ChangedPlayername moving with a speed 4.09112. Expected speed: 4 Distance: 11.6414
[07-02-2021 :: 07:43:20] [Zone] [Cheat] ChangedPlayername moving with a speed 4.06434. Expected speed: 4 Distance: 11.7019
[07-02-2021 :: 07:43:37] [Zone] [Cheat] ChangedPlayername moving with a speed 4.14357. Expected speed: 4 Distance: 12.2082
[07-02-2021 :: 07:43:53] [Zone] [Cheat] ChangedPlayername moving with a speed 4.13082. Expected speed: 4 Distance: 12.2923
[07-02-2021 :: 07:43:59] [Zone] [Cheat] ChangedPlayername moving with a speed 4.16087. Expected speed: 4 Distance: 14.4881
[07-02-2021 :: 07:44:17] [Zone] [Cheat] ChangedPlayername moving with a speed 4.00521. Expected speed: 4 Distance: 30.823

@dencelle
Copy link
Contributor Author

dencelle commented Jul 2, 2021 via email

@dencelle
Copy link
Contributor Author

dencelle commented Jul 2, 2021 via email

@dencelle
Copy link
Contributor Author

dencelle commented Jul 2, 2021

I am getting false positives with players on Sirensbane. This is from oot:

[07-01-2021 :: 14:44:24] [Zone] [Cheat] /MQWarp (large warp detection) with location from x [-5288.92] y [251.61] z [4.58]
[07-01-2021 :: 14:44:24] [Zone] [Cheat] ChangedPlayerName moving with a speed 6.11957. Expected speed: 4 Distance: 18.2328
[07-01-2021 :: 14:44:27] [Zone] [Cheat] /MQWarp (large warp detection) with location from x [-5163.52] y [250.10] z [4.56]
[07-01-2021 :: 14:44:27] [Zone] [Cheat] ChangedPlayerName moving with a speed 6.10313. Expected speed: 4 Distance: 60.569
[07-01-2021 :: 14:44:30] [Zone] [Cheat] /MQWarp (large warp detection) with location from x [-4960.65] y [236.31] z [4.51]
[07-01-2021 :: 14:44:30] [Zone] [Cheat] ChangedPlayerName moving with a speed 6.09086. Expected speed: 4 Distance: 16.9379
[07-01-2021 :: 14:44:33] [Zone] [Cheat] /MQWarp (large warp detection) with location from x [-4800.88] y [224.84] z [4.59]
[07-01-2021 :: 14:44:33] [Zone] [Cheat] ChangedPlayerName moving with a speed 6.00146. Expected speed: 4 Distance: 62.4828
[07-01-2021 :: 14:44:36] [Zone] [Cheat] /MQWarp (large warp detection) with location from x [-4596.94] y [202.10] z [4.54]
[07-01-2021 :: 14:44:36] [Zone] [Cheat] ChangedPlayerName moving with a speed 6.10218. Expected speed: 4 Distance: 17.5864
[07-01-2021 :: 14:44:39] [Zone] [Cheat] /MQWarp (large warp detection) with location from x [-4437.24] y [189.23] z [4.61]
[07-01-2021 :: 14:44:39] [Zone] [Cheat] ChangedPlayerName moving with a speed 6.03667. Expected speed: 4 Distance: 62.5097

Is there a way to either exclude a zone completely or find out if players are boarded?

256ff56 this commit should of fixed the false postive you described btw. cx,cy,cz are calculated with boats right above this so you shouldn't have that issue...

@ghost
Copy link

ghost commented Jul 3, 2021

I am getting false positives with players on Sirensbane. This is from oot:
[07-01-2021 :: 14:44:24] [Zone] [Cheat] /MQWarp (large warp detection) with location from x [-5288.92] y [251.61] z [4.58]
[07-01-2021 :: 14:44:24] [Zone] [Cheat] ChangedPlayerName moving with a speed 6.11957. Expected speed: 4 Distance: 18.2328
[07-01-2021 :: 14:44:27] [Zone] [Cheat] /MQWarp (large warp detection) with location from x [-5163.52] y [250.10] z [4.56]
[07-01-2021 :: 14:44:27] [Zone] [Cheat] ChangedPlayerName moving with a speed 6.10313. Expected speed: 4 Distance: 60.569
[07-01-2021 :: 14:44:30] [Zone] [Cheat] /MQWarp (large warp detection) with location from x [-4960.65] y [236.31] z [4.51]
[07-01-2021 :: 14:44:30] [Zone] [Cheat] ChangedPlayerName moving with a speed 6.09086. Expected speed: 4 Distance: 16.9379
[07-01-2021 :: 14:44:33] [Zone] [Cheat] /MQWarp (large warp detection) with location from x [-4800.88] y [224.84] z [4.59]
[07-01-2021 :: 14:44:33] [Zone] [Cheat] ChangedPlayerName moving with a speed 6.00146. Expected speed: 4 Distance: 62.4828
[07-01-2021 :: 14:44:36] [Zone] [Cheat] /MQWarp (large warp detection) with location from x [-4596.94] y [202.10] z [4.54]
[07-01-2021 :: 14:44:36] [Zone] [Cheat] ChangedPlayerName moving with a speed 6.10218. Expected speed: 4 Distance: 17.5864
[07-01-2021 :: 14:44:39] [Zone] [Cheat] /MQWarp (large warp detection) with location from x [-4437.24] y [189.23] z [4.61]
[07-01-2021 :: 14:44:39] [Zone] [Cheat] ChangedPlayerName moving with a speed 6.03667. Expected speed: 4 Distance: 62.5097
Is there a way to either exclude a zone completely or find out if players are boarded?

256ff56 this commit should of fixed the false postive you described btw. cx,cy,cz are calculated with boats right above this so you shouldn't have that is
Ya, the boat issue is fixen. Thank you for this!

I am getting false positives with players on Sirensbane. This is from oot:
[07-01-2021 :: 14:44:24] [Zone] [Cheat] /MQWarp (large warp detection) with location from x [-5288.92] y [251.61] z [4.58]
[07-01-2021 :: 14:44:24] [Zone] [Cheat] ChangedPlayerName moving with a speed 6.11957. Expected speed: 4 Distance: 18.2328
[07-01-2021 :: 14:44:27] [Zone] [Cheat] /MQWarp (large warp detection) with location from x [-5163.52] y [250.10] z [4.56]
[07-01-2021 :: 14:44:27] [Zone] [Cheat] ChangedPlayerName moving with a speed 6.10313. Expected speed: 4 Distance: 60.569
[07-01-2021 :: 14:44:30] [Zone] [Cheat] /MQWarp (large warp detection) with location from x [-4960.65] y [236.31] z [4.51]
[07-01-2021 :: 14:44:30] [Zone] [Cheat] ChangedPlayerName moving with a speed 6.09086. Expected speed: 4 Distance: 16.9379
[07-01-2021 :: 14:44:33] [Zone] [Cheat] /MQWarp (large warp detection) with location from x [-4800.88] y [224.84] z [4.59]
[07-01-2021 :: 14:44:33] [Zone] [Cheat] ChangedPlayerName moving with a speed 6.00146. Expected speed: 4 Distance: 62.4828
[07-01-2021 :: 14:44:36] [Zone] [Cheat] /MQWarp (large warp detection) with location from x [-4596.94] y [202.10] z [4.54]
[07-01-2021 :: 14:44:36] [Zone] [Cheat] ChangedPlayerName moving with a speed 6.10218. Expected speed: 4 Distance: 17.5864
[07-01-2021 :: 14:44:39] [Zone] [Cheat] /MQWarp (large warp detection) with location from x [-4437.24] y [189.23] z [4.61]
[07-01-2021 :: 14:44:39] [Zone] [Cheat] ChangedPlayerName moving with a speed 6.03667. Expected speed: 4 Distance: 62.5097
Is there a way to either exclude a zone completely or find out if players are boarded?

256ff56 this commit should of fixed the false postive you described btw. cx,cy,cz are calculated with boats right above this so you shouldn't have that issue...

The boat issue is fixed. Thanks!!!

@Dwordis
Copy link
Contributor

Dwordis commented Jul 5, 2021

Implemented this on my server and had some crashing when using the teleporter pads around erudin.

@SecretsOTheP
Copy link
Contributor

SecretsOTheP commented Jul 6, 2021 via email

@Dwordis
Copy link
Contributor

Dwordis commented Jul 7, 2021

Do you have any stack traces of the crashes?

On Mon, Jul 5, 2021 at 4:06 PM Gangsta @.***> wrote: Implemented this on my server and had some crashing when using the teleporter pads around erudin. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#1434 (comment)>, or unsubscribe https://github.com/notifications/unsubscribe-auth/AACOAQAEBOGPEJJGA4PKUGLTWIGE5ANCNFSM467MJ2XA .

Negative, removed it quickly was just commenting on my experience with the pr.

@dencelle
Copy link
Contributor Author

dencelle commented Jul 7, 2021

well... crap... i can't get it to replicate and no stack traces.... to figure out what the problem could be...

zone/client_packet.cpp Outdated Show resolved Hide resolved
zone/spells.cpp Outdated Show resolved Hide resolved
@Akkadius
Copy link
Member

Akkadius commented Jul 8, 2021

One thing we're going to likely need with a PR like this is some sort of controlled roll-out plan. Meaning, it needs to be tested with a size-able audience to ensure crash resiliency considering most of this functionality is on by default

Typically what I would do for a sensitive branch is run it in isolation on PEQ test server and then subsequently the live server for a period of time and monitor before merging into master.

If we have speculative crashes I would like us to at least do that to ensure we've covered our bases

We don't want this PR sitting in perpetuity but we also don't want to merge this; have a bunch of issues and then back out the entire PR

Lots of changes have been made to this active PR since its been made so would at least like to see some form of proof that things have been tested thoroughly and we have confidence to merge

Also, what is the plan to monitor this and tweak this into the future? Is there enough automatically recorded data that if given samples from a server we will be able to continuously tweak false positives to increase trust in the various detection methods

@dencelle
Copy link
Contributor Author

dencelle commented Jul 9, 2021

One thing we're going to likely need with a PR like this is some sort of controlled roll-out plan. Meaning, it needs to be tested with a size-able audience to ensure crash resiliency considering most of this functionality is on by default

Typically what I would do for a sensitive branch is run it in isolation on PEQ test server and then subsequently the live server for a period of time and monitor before merging into master.

If we have speculative crashes I would like us to at least do that to ensure we've covered our bases

We don't want this PR sitting in perpetuity but we also don't want to merge this; have a bunch of issues and then back out the entire PR

Lots of changes have been made to this active PR since its been made so would at least like to see some form of proof that things have been tested thoroughly and we have confidence to merge

Also, what is the plan to monitor this and tweak this into the future? Is there enough automatically recorded data that if given samples from a server we will be able to continuously tweak false positives to increase trust in the various detection methods

i'd love to see a bigger reliable server that i could get stack traces on so i can find any potential flaws, i have this running on my server but i haven't had any crashes related to this code thats being implemented. i like the idea of forwarding it to PEQ test for more testing and assurance that we don't have any crashes. i'm a speculating gangsta's crash was possibly due to not merging in correctly but i'm unsure since he wasn't able to produce a stack trace.

as for false positives, all warping gives from and to points now in logs as of last commit, if given the data i can re-create the movement and see if something is triggering a false positive but i've gone through all zones from classic to pop and have tried everything i can think of to cause a false postive, currently nothing is coming up.

basic summery: if i'm given enough info, i can most likely identify if its a false positive or not; if its a crash (as any crash goes) if given a stack trace, i can identify where the crash came from.

@Akkadius
Copy link
Member

We can run this through PEQ test easy enough; you'll need to rebase your changes again

Probably line this up during a weekend and get some testers going. ProjectEQ players have always been helpful with testing changes

@dencelle
Copy link
Contributor Author

dencelle commented Jul 27, 2021 via email

@SecretsOTheP
Copy link
Contributor

SecretsOTheP commented Jul 27, 2021 via email

zone/cheat_manager.cpp Outdated Show resolved Hide resolved
zone/cheat_manager.cpp Outdated Show resolved Hide resolved
zone/cheat_manager.cpp Outdated Show resolved Hide resolved
common/ruletypes.h Outdated Show resolved Hide resolved
@Akkadius Akkadius merged commit 7b069dc into EQEmu:master Aug 31, 2021
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.

5 participants