Skip to content

fix(telnet): stop a short NAWS report from crashing the server - #647

Merged
MorquinDevlar merged 1 commit into
masterfrom
fix/naws-short-payload-panic
Oct 5, 2026
Merged

MorquinDevlar merged 1 commit into
masterfrom
fix/naws-short-payload-panic

Conversation

@MorquinDevlar

Copy link
Copy Markdown
Contributor

Description

Any client can crash the server before logging in by sending a 3-byte NAWS (window size) report: IAC SB NAWS 00 50 00.

TelnetParseScreenSizePayload checks len(info) >= 3 and then reads info[3], which panics with index out of range [3] with length 3. The parser runs on the connection goroutine, and that goroutine has no recover, so the whole process exits and every connected player is dropped. The off-by-one dates back to the initial commit.

TelnetIACHandler does not buffer partial subnegotiations across reads. A normal client whose NAWS reply is split across two reads therefore produces the same short payload.

Changes

  • internal/term/telnet.go: TelnetParseScreenSizePayload requires 4 bytes. A shorter report returns the existing error. The caller already handles that error by logging it and keeping the current screen size.
  • internal/term/telnet_test.go (new): table test for the parser.
    • Parse: a 4-byte report, and a 6-byte report that keeps the trailing IAC SE the matcher leaves in.
    • Return an error: 3 bytes, 2 bytes, an empty payload and nil.
  • internal/inputhandlers/term_iac_test.go: TestTelnetIACHandlerShortScreenSizeReport sends the 3-byte report through TelnetIACHandler and asserts that it does not panic.

Test plan

  • Both new tests fail on master with the panic above and pass with the fix.
  • make validate passes.
  • go test -race ./... passes.

Not in this PR: the connection goroutines (handleTelnetConnection and the others) still have no panic guard, so a panic in any other input handler will also end the process.

TelnetParseScreenSizePayload checked for at least 3 bytes and then read
4. A NAWS report with only 3 bytes of payload read past its end and
panicked on the connection goroutine, which has no recover, so the whole
server exited. Any client could trigger it before logging in.

Require 4 bytes. Add a table test for the parser and a regression test
that sends the short report through TelnetIACHandler.
@MorquinDevlar
MorquinDevlar requested a review from Volte6 as a code owner October 4, 2026 09:22
@MorquinDevlar
MorquinDevlar merged commit b004870 into master Oct 5, 2026
11 checks passed
@MorquinDevlar
MorquinDevlar deleted the fix/naws-short-payload-panic branch October 5, 2026 05:24
pruuk added a commit to pruuk/DOGMud that referenced this pull request Oct 5, 2026
fix(telnet): port upstream GoMudEngine#647, short NAWS report no longer panics
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants