From edf8abd69e1c0df1004f23db509be80ddb1166e4 Mon Sep 17 00:00:00 2001 From: Brendan Creane Date: Mon, 5 Oct 2026 12:29:40 -0700 Subject: [PATCH] ipn/conffile: reject null services and endpoints in Services config files (#21647) A JSON null for a service or for an endpoint target unmarshals as a nil pointer, and loadConfigV0 dereferenced it without a check. This made "tailscale serve set-config" panic. Return an error that names the service or endpoint instead. Fixes #21534 Signed-off-by: Brendan Creane --- ipn/conffile/serveconf.go | 6 +++++ ipn/conffile/serveconf_test.go | 43 ++++++++++++++++++++++++++++++++++ 2 files changed, 49 insertions(+) diff --git a/ipn/conffile/serveconf.go b/ipn/conffile/serveconf.go index cff47d303..bd8d6c57f 100644 --- a/ipn/conffile/serveconf.go +++ b/ipn/conffile/serveconf.go @@ -261,6 +261,9 @@ func loadConfigV0(json []byte, forService string) (*ServicesConfigFile, error) { } } for svcName, svc := range scf.Services { + if svc == nil { + return nil, fmt.Errorf("service %q: must not be null", svcName) + } if forService == "" && svc.Version != "" { return nil, errors.New("services cannot be versioned separately from config file") } @@ -274,6 +277,9 @@ func loadConfigV0(json []byte, forService string) (*ServicesConfigFile, error) { foundTUN := false foundNonTUN := false for ppr, target := range svc.Endpoints { + if target == nil { + return nil, fmt.Errorf("service %q: endpoint %q: must not be null", svcName, ppr.String()) + } if target.Protocol == "TUN" { if ppr.Proto != 0 || ppr.Ports != tailcfg.PortRangeAny { return nil, fmt.Errorf("service %q: destination \"TUN\" can only be used with source \"*\"", svcName) diff --git a/ipn/conffile/serveconf_test.go b/ipn/conffile/serveconf_test.go index 6b1cc2b77..21383e2f1 100644 --- a/ipn/conffile/serveconf_test.go +++ b/ipn/conffile/serveconf_test.go @@ -6,6 +6,8 @@ package conffile import ( + "os" + "path/filepath" "testing" "tailscale.com/tailcfg" @@ -106,3 +108,44 @@ func TestTargetUnixSocketRoundtrip(t *testing.T) { }) } } + +func TestLoadServicesConfigNull(t *testing.T) { + tests := []struct { + name string + forService string + config string + wantErr string + }{ + { + name: "null_service", + config: `{"version":"0.0.1","services":{"svc:a":null}}`, + wantErr: `service "svc:a": must not be null`, + }, + { + name: "null_endpoint", + config: `{"version":"0.0.1","services":{"svc:a":{"endpoints":{"tcp:443":null}}}}`, + wantErr: `service "svc:a": endpoint "tcp:443": must not be null`, + }, + { + name: "null_endpoint_for_service", + forService: "svc:a", + config: `{"version":"0.0.1","endpoints":{"tcp:443":null}}`, + wantErr: `service "svc:a": endpoint "tcp:443": must not be null`, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + path := filepath.Join(t.TempDir(), "config.json") + if err := os.WriteFile(path, []byte(tt.config), 0o600); err != nil { + t.Fatal(err) + } + _, err := LoadServicesConfig(path, tt.forService) + if err == nil { + t.Fatalf("LoadServicesConfig succeeded; want error %q", tt.wantErr) + } + if err.Error() != tt.wantErr { + t.Errorf("LoadServicesConfig error = %q; want %q", err, tt.wantErr) + } + }) + } +}