Skip to content

DoS Fixes - #2866

Merged
Z1Meme merged 2 commits into
devfrom
dos-fixes
Aug 22, 2026
Merged

Z1Meme merged 2 commits into
devfrom
dos-fixes

Conversation

@jamieduke

@jamieduke jamieduke commented Aug 7, 2026 •

Copy link
Copy Markdown
Collaborator

Summary

The SOE layer can be wedged or exhausted by unauthenticated UDP traffic. This fixes three issues that are each independently sufficient to take a login or zone server down, plus a set of related hardening fixes in the same code paths.

This started from a login server log captured during a live attack. The visible symptom — process alive, socket bound, no new connections accepted — is consistent with event-loop starvation rather than the port exhaustion we initially assumed. There's only one UDP socket, so ports were never the constraint.

The three primary issues

1. SessionRequest.udp_length can hang the process indefinitely

handlePacket took udp_length straight off the wire and used it to derive the output stream's fragment size:

client.clientUdpLength = packet.udp_length;
client.outputStream.setFragmentSize(client.clientUdpLength - (4 + this._crcLength));

Any value below 6 makes the fragment size zero or negative. writeReliable() then takes its fragmentation branch (data.length <= fragmentSize is never true) and steps the loop counter by a non-positive amount, so it never terminates — spinning forever while pushing an entry into the resend cache on every iteration.

The event loop blocks permanently. The process stays alive and the socket stays bound, which is exactly the observed symptom. This needs no volume and no authentication.

Fix: clamp udp_length to [512, 65535]. setFragmentSize() and writeReliable() now also reject a non-positive fragment size as a backstop — worth noting the default _fragmentSize is 0, so that loop was reachable before a session was ever established.

2. Unauthenticated packets could create permanently-leaked clients

onMessage created a client based on data[1] === 1 alone, before parsing anything. A 2-byte datagram from a fresh source port allocated a full SOEClient with a 24ms sending interval.

Worse, the keep-alive timer was armed only inside the SessionRequest handler. A client created this way never reached that code, so it was never expired and never deleted. Each junk packet bought a permanent 24ms timer.

The session rate limiter caps this at 5/IP/min, but the attack log shows 206 distinct source IPs — roughly 1,000 permanently-leaked timers per minute. And since a client is created before any handshake, source addresses are spoofable, so that limiter is bypassable in principle.

Fix: parse first, and only create a client for a packet that actually decodes as a SessionRequest. Arm the keep-alive in _createClient() so every client expires regardless of how far it got.

Reviewers, please look closely here: the keep-alive timeout now calls deleteClient() itself before emitting disconnect. This is deliberate. GatewayServer only re-emits client.sessionId upward, and ZoneServer2016 resolves that against this._clients[sessionId]. A half-open client has sessionId = 0, so that lookup misses and nothing ever deletes the SOE client. Emitting alone would not have been enough on the zone path.

3. deleteClient() leaked a timer and the whole client with it

It cleared the sending interval but never called closeTimers(), so _statsResetTimer kept running. Its closure retains the client — input stream, output stream, _appDataMap, RC4 state.

This one leaks on the normal path too: every legitimate logout. soeserver.test.ts needing an after() hook that force-calls process.exit(0) is the tell.

Fix: deleteClient() calls client.closeTimers(), which now clears all three timers.

Additional hardening

All reachable from unauthenticated input, in the same code paths:

Issue Fix
Fragment reassembly allocated a Buffer sized by an attacker-supplied uint32 — up to 4 GiB from one packet Capped at MAX_REASSEMBLY_SIZE, plus a header-length guard. The existing overflow check now returns instead of emitting an error and continuing.
ack() walked the sequence space from lastAck — up to 65,535 cache lookups for one crafted ack Bounded by maxSequenceAvailable. Also removes a latent infinite loop: wrappedUint16.wrap() only subtracts once, so a large enough value could never be reached by incrementing.
_appDataMap buffered out-of-order packets without limit Capped at MAX_BUFFERED_PACKETS.
_processData() recursed once per buffered packet Rewritten iteratively; same logic, no stack growth.
Rate-limit maps have 2s/62s TTLs but were swept every 60s, so a spoofed-source flood grew them unbounded between sweeps Swept every 5s. The handle is now assigned to _packetResetInterval, which stop() already tried to clear but which was never actually set.
process.exitCode = 1 in the onMessage catch Removed. Any malformed packet silently marked the process as failed for its eventual exit, which would confuse pm2/Docker restart policy.
JSON.parse of the h1emu-core output was unguarded Wrapped; the packet is dropped instead.

That last one is what appears in the logs — h1emu-core 1.3.2 returned malformed JSON (SyntaxError at position 114) for hostile input, throwing out of onMessage. It was being caught, so it isn't the outage, but it's a parser bug that should be fixed in h1emu-core separately — this PR only stops it from propagating.

Verification

tsc --noEmit clean, oxlint clean, existing soeserver.test.ts passes.

Beyond that I wrote a script that exercises each fix against the real classes. Before the changes it confirmed all three primary issues reproduce; after, all 13 checks pass:

  • 2-byte junk and unparseable SessionRequests create 0 clients (was 5 and 1)
  • keep-alive and sending interval both armed at _createClient
  • udp_length = 0 yields fragment size 506, and writeReliable terminates (was -6, unbounded — a guard tripped at 2,000,001 iterations)
  • all three timers destroyed after deleteClient
  • a far-ahead ack advances lastAck by 0
  • a 4 GiB reassembly request is rejected with RSS growth of 0.0 MB
  • the out-of-order buffer caps at 1024

I can fold this into soeserver.test.ts as proper regression tests if you'd like it in the suite.

Notes for reviewers

  • MAX_REASSEMBLY_SIZE (16 MiB) and MAX_BUFFERED_PACKETS (1024) are my best guesses. You guys know the real traffic, It's been a really long time since I ran a server.
  • setFragmentSize() now throws. With the clamp in place it shouldn't ever fire, and a throw from handlePacket lands in onMessage's catch and drops the packet. Flagging it in case anything constructs an SOEOutputStream and sets a fragment size directly.
  • Behaviour change: unknown clients sending raw (non-SOE) data are now dropped rather than implicitly creating a session. This only matters if _allowRawDataReception is enabled, and it's the intended hardening, but it is a semantic change.
  • The soeclient.ts change was already committed to this branch separately.

(Co-authored with Fable 5)

@github-actions github-actions Bot added the core Need to modify core stuff ( soe server ) label Aug 7, 2026
@jasonmoonen
jasonmoonen changed the base branch from master to dev August 8, 2026 09:33
@Z1Meme
Z1Meme marked this pull request as ready for review August 22, 2026 04:52
@Z1Meme
Z1Meme self-requested a review August 22, 2026 04:52
@Z1Meme
Z1Meme merged commit 559fc79 into dev Aug 22, 2026
20 checks passed
@Z1Meme
Z1Meme deleted the dos-fixes branch August 22, 2026 05:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

core Need to modify core stuff ( soe server )

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants