openspy / openspy-core

132 stars 21 forks source link

I think I found a bug in "gamespy3dxor" function and gamestats accumulator #4

Closed HerMajestyDrMona closed 3 years ago

HerMajestyDrMona commented 3 years ago

As first, I'd like to apologize if my thoughts are incorrect. I would just like to share some observations.

I'm using the old openspy-core code for my "little" project. A few years ago, I detected that function "gamespy3dxor" works incorrectly. Generally, the game sends a very long requests to get data of ~200 players for displaying Leaderboards. I made my own accumulator to handle unfinished requests, but something was still working incorrectly. Most of problems were fixed by my simple "fix": https://github.com/chc/openspy-core-v2/blob/master/gamestats/server/GSPeer.cpp#L567 I noticed that changing this line "len-=7;" to "len-=6;" prevented junk data at the end of request buffers.

Example with -= 7: https://i.imgur.com/huKQMdW.png

Example with -= 6: https://i.imgur.com/3GoB8fF.png

Second problem is with "m_kv_accumulator": https://github.com/chc/openspy-core-v2/blob/master/gamestats/server/GSPeer.cpp#L82 It connects strings after gamespy3dxor decryption. It might work, but in certain cases the data might be split in the middle of the "\final\" text.

Example: Incoming data1: \getpd\pid\1093\ptype\2\dindex\0\keys\lid\0\fi Incoming data2: nal\getpd\pid\1094\ptype\2\dindex\0\keys\lid\0\final\

Since gamespy3dxor can't find the full "\final\" it thinks that it's a normal data to decrypt. The solution is to use the encrypted data in the accumulator (before gamespy3dxor function was called).

My C++ skills are rather simple, so I can only provide a pseudo-code of my fix, but I hope it will be helpful: https://pastebin.com/JXBhpDtw

chc commented 3 years ago

Thanks for the ticket, I will review this and check further. Please not the code has just been resynced and updated, its possible this bug has been solved by now.