DoS Exploit patch

[HacksLua]

Fix: DoS via malicious encrypted packets

Problem

The server accepted UDP packets from clients that had completed the Lidgren handshake, but whose contents were forged or intentionally corrupted. NetEncryption.Decrypt() threw a SodiumException that was not caught anywhere in the call chain:

NetEncryption.Decrypt()          < exception thrown here
NetManager.DispatchNetMessage()  < not caught
NetManager.ProcessPackets()      < not caught
BaseServer.Input()               < not caught
GameLoop.Run()                   < caught here - too late

GameLoop logged the error and continued, but all packets remaining in the current tick were dropped - legitimate clients stopped receiving state updates and disconnected with FailedNotConnected.

This allowed a single attacker witgh an established Lidgren connection to continuously kick all players from the server by flooding it with corrupted packets.

Changes

Robust.Shared/Network/NetEncryption.cs

• Decrypt() now returns bool instead of throwing SodiumException on MAC failure. A bad packet is now a branch, not an exception.

Robust.Shared/Network/NetManager.cs

• Replaced try/catch with if (!channel.Encryption.Decrypt(msg)).
• Added _decryptFailCounts - tracks failures per IP. IPv6 addresses are masked to /64 to prevent subnet hopping. Failure count never resets on cleanup to prevent threshold bypass by waiting.
• Logging only on first failure and at threshold to prevent log I/O DoS.
• Dictionary capped at net.dos_fail_max_tracked to prevent mewmory exhaustion from botnets.

Robust.Shared/CVars.cs

• net.dos_fail_kick (default: true) - disconnect upon reachimg threshold.
• net.dos_fail_ban_threshold (default: 10) - failures before [DECRYPTBAN] warning and optional kick.
• net.dos_fail_cleanup_interval (default: 2) - minutes between cleanup passes.
• net.dos_fail_max_tracked (default: 10000) - max IPs tracked simultaneously.

Before / Aftewr

	Before	After
Malicious packet	Exception escapes to GameLoop, all clients dropped for the tick	Attacker disconnected, other clients unaffected
Log output	[ERRO] runtime: Caught exception in "gameLoop Input"	[WARN] net: X.X.X.X: First decryption failure
Repeated attacks	Every packet crashes the tick indefinitely	Counter accumulates; at threshold [DECRYPTBAN] is emitted

Why this matters

• The attack requires no authentication - completing the Lidgren handshake is sufficient
• A single connection and minimal traffic is enough to sustain the attack
• The effect is a full drop of all online players on every malicious packet
• The fix removes the exception path entirely, making the hot path allocation-free

** Media
Before:

After

Server after patch don't falling from attacks

[KingPidor]

GOIDA

[Tostiss14]

vouch, kills servers in an instant
major issue

[codebykoi]

There is some flaws, such as memory leaks when registering IPs. Attackers can just spoof IP address, use botnets and such.
Dictionary is not thread safe, use ConcurrentDictionary<TKey, TValue> instead

[codebykoi]

You are also kicking users on first decrypt failure, your variable only works on logging.

[codebykoi]

Hard coding config variables isn't a best option, consider using it as CVar.

[HacksLua]

Ok

[HacksLua]

I'm checking now, but I don't have the tool to attack myself

[moonheart08]

Exceptions are themselves an extremely easy DoS vector, consider actually fixing Decrypt to not throw upon receiving this kind of malformed packet, instead.

[HacksLua]

Exceptions are themselves an extremely easy DoS vector, consider actually fixing Decrypt to not throw upon receiving this kind of malformed packet, instead.

I'll figure it out now...
pupu

[HacksLua]

Exceptions are themselves an extremely easy DoS vector, consider actually fixing Decrypt to not throw upon receiving this kind of malformed packet, instead.

done

[deathride58]

(Not an engine maint, so this comment does not serve as review.)

The approach in this PR appears to add an upfront O(n) processing cost to every packet the server receives, which massively amplifies packet spam, serving as its own DoS vector. This means that if this PR were merged with the approach that it's currently taking, malicious actors, in theory, would be able to take down servers by simply flooding empty packets at them (which is, frankly, much cheaper for malicious actors to exploit than the current DoS exploit is)

This PR additionally fails to address the actual root cause of the DoS exploit it's attempting to fix, going so far as to remove the exception entirely instead of addressing the bug that leads to the exception.

[HacksLua]

(Not an engine maint, so this comment does not serve as review.)

The approach in this PR appears to add an upfront O(n) processing cost to every packet the server receives, which massively amplifies packet spam, serving as its own DoS vector. This means that if this PR were merged with the approach that it's currently taking, malicious actors, in theory, would be able to take down servers by simply flooding empty packets at them (which is, frankly, much cheaper for malicious actors to exploit than the current DoS exploit is)

This PR additionally fails to address the actual root cause of the DoS exploit it's attempting to fix, going so far as to remove the exception entirely instead of addressing the bug that leads to the exception.

CleanupDecryptFailCounts exits immediately via a single DateTime comparison on every call the dictionary iteration only runs once every N minutes, so per-packet cost is O(I), not O(n).
On root cause: Sodium already returns bool from its decrypt call - we were just wrapping it in an unnecessary throw. Returning bool from Decrypt() instead of throwing is the fix at the source.

[HacksLua]

Either way, I've deployed it to my own server and will be rolling it out to a few others soon for broader testing

[codebykoi]

(Not an engine maint, so this comment does not serve as review.)

The approach in this PR appears to add an upfront O(n) processing cost to every packet the server receives, which massively amplifies packet spam, serving as its own DoS vector. This means that if this PR were merged with the approach that it's currently taking, malicious actors, in theory, would be able to take down servers by simply flooding empty packets at them (which is, frankly, much cheaper for malicious actors to exploit than the current DoS exploit is)

This PR additionally fails to address the actual root cause of the DoS exploit it's attempting to fix, going so far as to remove the exception entirely instead of addressing the bug that leads to the exception.

CleanupDecryptFailCounts exits immediately via a single DateTime comparison on every call the dictionary iteration only runs once every N minutes, so per-packet cost is O(I), not O(n).
On root cause: Sodium already returns bool from its decrypt call - we were just wrapping it in an unnecessary throw. Returning bool from Decrypt() instead of throwing is the fix at the source.

the PR author has a valid point, tho it still would be nice to see some benchmarks on how it affects the CPU performance

[codebykoi]

I think #6491
fixes the original issue

[HacksLua]

Moving to Draft - the original exploit is fixed by #6491. This PR adds additional botnet mitigation on top: per-IP failure tracking (IPv6 masked to /64), a memory cap on tracked IPs, throttled logging to prevent log I/O DoS, and a [DECRYPTBAN] warning for automated IP blocking. Useful if you're dealing with targeted botnet attacks, but not critical for most servers.