aboutsummaryrefslogtreecommitdiff
path: root/hotline/file_transfer.go
diff options
context:
space:
mode:
authorJeff Halter <868228+jhalter@users.noreply.github.com>2026-05-31 14:48:31 -0700
committerJeff Halter <868228+jhalter@users.noreply.github.com>2026-05-31 14:48:31 -0700
commit123d1a305bc68474034f5989362148508bdbf7f5 (patch)
tree341a0030c5a1a9abfaa73fa31bd59c7e5c216bf7 /hotline/file_transfer.go
parent588dce918aeda0efc4db80b30525d28943c029cd (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.go6
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