From 123d1a305bc68474034f5989362148508bdbf7f5 Mon Sep 17 00:00:00 2001
From: Jeff Halter <868228+jhalter@users.noreply.github.com>
Date: Sun, 31 May 2026 14:48:31 -0700
Subject: Validate client input to prevent panics from malformed transactions

A malicious or buggy client could send transaction fields with the wrong
length and trigger runtime panics (slice/index out of range, slice-to-array
conversion, nil deref) in the parsing and handler code. These were caught by
the connection-level recover, so they dropped the client connection and dumped
a stack trace to stdout rather than crashing the process, but they are still
incorrect behavior, a log-flood vector, and a latent crash if the recover
scope ever changes.

Fix at the source and harden the safety net:

- Add ClientIDFromBytes / ChatIDFromBytes helpers that return ok=false on a
  length mismatch, and use them in the transaction handlers instead of direct
  [2]byte(...) / [4]byte(...) conversions on field data. Nil-check ClientMgr.Get
  results, and length-guard FieldOptions and the HandleUpdateUser sub-field
  header. Malformed input now yields an error reply (or a clean no-op for
  reply-less handlers) instead of panicking.
- Bounds-check FileResumeData.UnmarshalBinary (header length and fork count)
  and guard the ForkInfoList[0] accesses against an empty list.
- Bounds-check FlatFileInformationFork parsing (reachable on upload): validate
  the fixed header, name, and comment lengths, and fix a latent 72+nameSize
  uint16 overflow. Route Write through UnmarshalBinary.
- panic.go: stop printing stack traces to stdout (keep structured logging) so a
  client cannot flood stdout by repeatedly triggering a panic.
- handleTransaction: recover per-transaction so one malformed request no longer
  tears down the whole client connection.

Adds tests for the new helpers, the hardened resume-data and flat-file-object
decoders, and handler-level malformed-ID handling.
---
 hotline/panic.go | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

(limited to 'hotline/panic.go')

diff --git a/hotline/panic.go b/hotline/panic.go
index d7376db..2207911 100644
--- a/hotline/panic.go
+++ b/hotline/panic.go
@@ -1,15 +1,16 @@
 package hotline
 
 import (
-	"fmt"
 	"log/slog"
 	"runtime/debug"
 )
 
-// dontPanic logs panics instead of crashing
+// dontPanic recovers from a panic and logs it (with a stack trace) instead of
+// letting it crash the goroutine. The trace is recorded via the structured
+// logger only; it is intentionally not written to stdout, so a client that can
+// repeatedly trigger a panic cannot flood stdout.
 func dontPanic(logger *slog.Logger) {
 	if r := recover(); r != nil {
-		fmt.Println("stacktrace from panic: \n" + string(debug.Stack()))
 		logger.Error("PANIC", "err", r, "trace", string(debug.Stack()))
 	}
 }
-- 
cgit