Pryaxis / TShock

☕️⚡️TShock provides Terraria servers with server-side characters, anti-cheat, and community management tools.
GNU General Public License v3.0
2.43k stars 382 forks source link

Server "Crash" Exploit #1655

Closed tysonstrange closed 5 years ago

tysonstrange commented 5 years ago

Reproduction steps (if applicable)?

Unsure, but I have a pcap from wireshark of it happening.

Any stack traces or error messages (if known)?

All plugins start throwing an OnGetData exceptions, but it spams console like this: https://imgur.com/a/gmUjjtC

Seen some kid logging in and doing something on purpose to trigger it.

Let me know if you want a copy of this pcap and filter.

sr229 commented 5 years ago

@tysonstrange Security vulnerabilities shouldn't be disclosed publicly until it has been patched already. You should've emailed the author to prevent anyone from exploiting the vulnerability further.

ivanbiljan commented 5 years ago

The issue does not contain any critical information, so the vulnerability hasn't been publicly disclosed. @tysonstrange you can/should probably forward what you have to the admins, either via discord or telegram.

tysonstrange commented 5 years ago

Turns out it's been bandaide patched already.

https://github.com/Pryaxis/TShock/tree/0-length-temp-fix

Close this off.

hakusaro commented 5 years ago

@sr229 thanks 💙!

On Thu, Jan 10, 2019 at 02:30 tysonstrange notifications@github.com wrote:

Turns out it's been bandaide patched already.

https://github.com/Pryaxis/TShock/tree/0-length-temp-fix

Close this off.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/Pryaxis/TShock/issues/1655#issuecomment-453047367, or mute the thread https://github.com/notifications/unsubscribe-auth/AAggpxJ9ucN5TGIOwBm2qMJmPST9xJRmks5vBxY2gaJpZM4Z4qCs .

-- Lucas Nicodemus http://keybase.io/xn

tysonstrange commented 5 years ago

Well, actually. Not "Fixed", the DOS can still occur, this just catches and stops exceptions.

It still locks up/slows down/ freezes the server.

Looks like it lucks up until the connection times out and the stream disposes.

Only noticed it do this because it threw and excetpioon on a disposed object

sr229 commented 5 years ago

TShock versions affected is probably v4.3.25 and below. Security patch is required.

tysonstrange commented 5 years ago

is there perhaps anythign we can do from this end to mitigate it

Seems that extra check is not enough blob:https://imgur.com/3340e89b-4ed2-4adb-a646-f7b338a9cdc2

bartico6 commented 5 years ago

wasn't this problem already addressed in Pryaxis/TerrariaAPI-Server@8e6a6ae72df18d2e92c9a69fdc45246ff25f4bc6 by @QuiCM ?

Ideally the server shouldn't log those because logging is what causes slowdowns in the first place. Rapidly writing logs causes the server to repeatedly acquire and free the Console lock anyway....

tylerjwatson commented 5 years ago

Ideally the server shouldn't log those because logging is what causes slowdowns in the first place. Rapidly writing logs causes the server to repeatedly acquire and free the Console lock anyway....

That's an excellent point. Should also check that there aren't hidden exceptions further in.

bartico6 commented 5 years ago

To expand upon this point; the server shouldn't do any expensive operations (such as console logging, and, by consequence, file logging) because of someone trying to DoS it - that's creating the DoS surface you're trying to minimize.

Just don't log these, or make logging an inexpensive operation.

QuiCM commented 5 years ago

The above commit removes the logging Thanks

hakusaro commented 5 years ago

@QuiCM can this be closed?

QuiCM commented 5 years ago

Yep, my bad