Repository navigation
Conversation
actually clear the timers
Z1Meme
marked this pull request as ready for review
August 22, 2026 04:52
Z1Meme
self-requested a review
August 22, 2026 04:52
Z1Meme
approved these changes
Aug 22, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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_lengthcan hang the process indefinitelyhandlePackettookudp_lengthstraight off the wire and used it to derive the output stream's fragment size:Any value below
6makes the fragment size zero or negative.writeReliable()then takes its fragmentation branch (data.length <= fragmentSizeis 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_lengthto[512, 65535].setFragmentSize()andwriteReliable()now also reject a non-positive fragment size as a backstop — worth noting the default_fragmentSizeis0, so that loop was reachable before a session was ever established.2. Unauthenticated packets could create permanently-leaked clients
onMessagecreated a client based ondata[1] === 1alone, before parsing anything. A 2-byte datagram from a fresh source port allocated a fullSOEClientwith a 24ms sending interval.Worse, the keep-alive timer was armed only inside the
SessionRequesthandler. 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.3.
deleteClient()leaked a timer and the whole client with itIt cleared the sending interval but never called
closeTimers(), so_statsResetTimerkept 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.tsneeding anafter()hook that force-callsprocess.exit(0)is the tell.Fix:
deleteClient()callsclient.closeTimers(), which now clears all three timers.Additional hardening
All reachable from unauthenticated input, in the same code paths:
Buffersized by an attacker-supplied uint32 — up to 4 GiB from one packetMAX_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 fromlastAck— up to 65,535 cache lookups for one crafted ackmaxSequenceAvailable. Also removes a latent infinite loop:wrappedUint16.wrap()only subtracts once, so a large enough value could never be reached by incrementing._appDataMapbuffered out-of-order packets without limitMAX_BUFFERED_PACKETS._processData()recursed once per buffered packet_packetResetInterval, whichstop()already tried to clear but which was never actually set.process.exitCode = 1in theonMessagecatchJSON.parseof the h1emu-core output was unguardedThat last one is what appears in the logs — h1emu-core 1.3.2 returned malformed JSON (
SyntaxErrorat position 114) for hostile input, throwing out ofonMessage. 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 --noEmitclean,oxlintclean, existingsoeserver.test.tspasses.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:
_createClientudp_length = 0yields fragment size 506, andwriteReliableterminates (was-6, unbounded — a guard tripped at 2,000,001 iterations)deleteClientlastAckby 0I can fold this into
soeserver.test.tsas proper regression tests if you'd like it in the suite.Notes for reviewers
MAX_REASSEMBLY_SIZE(16 MiB) andMAX_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 fromhandlePacketlands inonMessage's catch and drops the packet. Flagging it in case anything constructs anSOEOutputStreamand sets a fragment size directly._allowRawDataReceptionis enabled, and it's the intended hardening, but it is a semantic change.soeclient.tschange was already committed to this branch separately.(Co-authored with Fable 5)