From 21cf5016d98ff568692e91751d021facbdfc6bb6 Mon Sep 17 00:00:00 2001
From: Jeff Halter <868228+jhalter@users.noreply.github.com>
Date: Fri, 29 May 2026 08:22:49 -0700
Subject: Guard against panics from unchecked type assertions

Replace two unchecked type assertions that could crash the server:

- server.go: the file-transfer logger built remoteAddr via a bare
  ctx.Value(contextKeyReq).(requestCtx) assertion, which panics if the
  context value is absent or the wrong type. Use the comma-ok form and
  degrade to an empty string.

- access.go: the legacy (< v0.17.0) AccessBitmap YAML array path used
  byte(v.(int)) with no element-type or bounds check, panicking on a
  non-int element or an array longer than the [8]byte bitmap. Iterate the
  slice from the type switch, bounds-check the index, and comma-ok each
  element, returning a wrapped error so a malformed user file fails to
  load loudly instead of crashing the server.

Add regression tests for the non-int and oversized legacy array cases.
---
 hotline/access.go      | 11 +++++++++--
 hotline/access_test.go | 10 ++++++++++
 hotline/server.go      |  6 +++++-
 3 files changed, 24 insertions(+), 3 deletions(-)

(limited to 'hotline')

diff --git a/hotline/access.go b/hotline/access.go
index 370f8c2..927e0a9 100644
--- a/hotline/access.go
+++ b/hotline/access.go
@@ -67,8 +67,15 @@ func (bits *AccessBitmap) UnmarshalYAML(unmarshal func(interface{}) error) error
 		// Mobius versions < v0.17.0 store the user access bitmap as an array of int values like:
 		// [96, 112, 12, 32, 3, 128, 0, 0]
 		// This case supports reading of user config files using this format.
-		for i, v := range flags.([]interface{}) {
-			bits[i] = byte(v.(int))
+		for i, elem := range v {
+			if i >= len(bits) {
+				return fmt.Errorf("unmarshal access bitmap: too many elements (%d, max %d)", len(v), len(bits))
+			}
+			n, ok := elem.(int)
+			if !ok {
+				return fmt.Errorf("unmarshal access bitmap: element %d is %T, want int", i, elem)
+			}
+			bits[i] = byte(n)
 		}
 	case map[string]interface{}:
 		// Mobius versions >= v0.17.0 store the user access bitmap as map[string]bool to provide a human-readable view of
diff --git a/hotline/access_test.go b/hotline/access_test.go
index 2c5b8f4..65d29be 100644
--- a/hotline/access_test.go
+++ b/hotline/access_test.go
@@ -74,6 +74,16 @@ func Test_accessBitmap_UnmarshalYAML(t *testing.T) {
 			expected: AccessBitmap{96, 112, 12, 32, 3, 128, 0, 0},
 			wantErr:  false,
 		},
+		{
+			name:     "legacy array with non-int element returns error",
+			yamlData: `access: [96, "nope", 12, 32, 3, 128, 0, 0]`,
+			wantErr:  true,
+		},
+		{
+			name:     "legacy array with too many elements returns error",
+			yamlData: "access: [1, 2, 3, 4, 5, 6, 7, 8, 9]",
+			wantErr:  true,
+		},
 		{
 			name: "unmarshal map format with true values",
 			yamlData: `access:
diff --git a/hotline/server.go b/hotline/server.go
index d757062..7bc6a4a 100644
--- a/hotline/server.go
+++ b/hotline/server.go
@@ -690,8 +690,12 @@ func (s *Server) handleFileTransfer(ctx context.Context, rwc io.ReadWriter) erro
 		time.Sleep(3 * time.Second)
 	}()
 
+	var remoteAddr string
+	if rc, ok := ctx.Value(contextKeyReq).(requestCtx); ok {
+		remoteAddr = rc.remoteAddr
+	}
 	rLogger := s.Logger.With(
-		"remoteAddr", ctx.Value(contextKeyReq).(requestCtx).remoteAddr,
+		"remoteAddr", remoteAddr,
 		"login", fileTransfer.ClientConn.Account.Login,
 		"Name", string(fileTransfer.ClientConn.UserName),
 	)
-- 
cgit