diff options
| author | Jeff Halter <868228+jhalter@users.noreply.github.com> | 2026-05-29 08:22:49 -0700 |
|---|---|---|
| committer | Jeff Halter <868228+jhalter@users.noreply.github.com> | 2026-05-29 08:22:49 -0700 |
| commit | 21cf5016d98ff568692e91751d021facbdfc6bb6 (patch) | |
| tree | f9a9ccfac5dd9252226aef84495f5aeb1cb7383f | |
| parent | b772019454ebb804c313e717ae98eb68430e780e (diff) | |
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.
| -rw-r--r-- | hotline/access.go | 11 | ||||
| -rw-r--r-- | hotline/access_test.go | 10 | ||||
| -rw-r--r-- | hotline/server.go | 6 |
3 files changed, 24 insertions, 3 deletions
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 @@ -75,6 +75,16 @@ func Test_accessBitmap_UnmarshalYAML(t *testing.T) { 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: DownloadFile: true 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), ) |