diff options
| author | Jeff Halter <868228+jhalter@users.noreply.github.com> | 2026-05-31 14:48:31 -0700 |
|---|---|---|
| committer | Jeff Halter <868228+jhalter@users.noreply.github.com> | 2026-05-31 14:48:31 -0700 |
| commit | 123d1a305bc68474034f5989362148508bdbf7f5 (patch) | |
| tree | 341a0030c5a1a9abfaa73fa31bd59c7e5c216bf7 /hotline/file_transfer.go | |
| parent | 588dce918aeda0efc4db80b30525d28943c029cd (diff) | |
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.
Diffstat (limited to 'hotline/file_transfer.go')
| -rw-r--r-- | hotline/file_transfer.go | 6 |
1 files changed, 4 insertions, 2 deletions
diff --git a/hotline/file_transfer.go b/hotline/file_transfer.go index e5898db..98999f1 100644 --- a/hotline/file_transfer.go +++ b/hotline/file_transfer.go @@ -246,7 +246,7 @@ func (fh *FileHeader) Read(p []byte) (int, error) { func DownloadHandler(w io.Writer, fullPath string, fileTransfer *FileTransfer, fs FileStore, rLogger *slog.Logger, preserveForks bool) error { var dataOffset int64 - if fileTransfer.FileResumeData != nil { + if fileTransfer.FileResumeData != nil && len(fileTransfer.FileResumeData.ForkInfoList) > 0 { dataOffset = int64(binary.BigEndian.Uint32(fileTransfer.FileResumeData.ForkInfoList[0].DataSize[:])) } @@ -458,7 +458,9 @@ func DownloadFolderHandler(rwc io.ReadWriter, fullPath string, fileTransfer *Fil if err := frd.UnmarshalBinary(resumeDataBytes); err != nil { return err } - dataOffset = int64(binary.BigEndian.Uint32(frd.ForkInfoList[0].DataSize[:])) + if len(frd.ForkInfoList) > 0 { + dataOffset = int64(binary.BigEndian.Uint32(frd.ForkInfoList[0].DataSize[:])) + } case DlFldrActionNextFile: // client asked to skip this file return nil |