From 0ac3bd0fb72701c77478bdd53d4551565bcc146b Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Manuel=20R=C3=BCger?= <manuel@rueg.eu>
Date: Wed, 2 Sep 2026 12:07:51 +0200
Subject: [PATCH] web: fix nil pointer dereference when systemd socket
 activation is disabled
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

checkFlags only required listen addresses to be set when WebSystemdSocket
was nil. A non-nil pointer to false satisfied the check, so a FlagConfig
with systemd socket activation explicitly disabled and no listen addresses
passed validation, and ListenAndServe then panicked dereferencing the nil
WebListenAddresses:

    listeners := make([]net.Listener, 0, len(*flags.WebListenAddresses))

kingpinflag.AddFlags always returns a non-nil WebSystemdSocket pointer, so
this is reachable from any caller that builds a FlagConfig directly rather
than through the kingpin flags.

Require listen addresses whenever systemd socket activation is not actually
enabled, and add tests for checkFlags, which was previously uncovered.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Signed-off-by: Manuel Rüger <manuel@rueg.eu>
---
 web/flags_test.go | 102 ++++++++++++++++++++++++++++++++++++++++++++++
 web/tls_config.go |  11 ++++-
 2 files changed, 111 insertions(+), 2 deletions(-)
 create mode 100644 web/flags_test.go

diff --git a/web/flags_test.go b/web/flags_test.go
new file mode 100644
index 00000000..e72c6d65
--- /dev/null
+++ b/web/flags_test.go
@@ -0,0 +1,102 @@
+// Copyright 2025 The Prometheus Authors
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package web
+
+import (
+	"errors"
+	"testing"
+)
+
+func TestCheckFlags(t *testing.T) {
+	for _, tc := range []struct {
+		name  string
+		flags *FlagConfig
+		want  error
+	}{
+		{
+			name:  "nil FlagConfig",
+			flags: nil,
+			want:  ErrMissingFlag,
+		},
+		{
+			name: "missing web config file",
+			flags: &FlagConfig{
+				WebListenAddresses: &[]string{":9100"},
+				WebSystemdSocket:   OfBool(false),
+			},
+			want: ErrMissingFlag,
+		},
+		{
+			name: "listen addresses only",
+			flags: &FlagConfig{
+				WebListenAddresses: &[]string{":9100"},
+				WebConfigFile:      OfString(""),
+			},
+			want: nil,
+		},
+		{
+			name: "systemd socket activation enabled without listen addresses",
+			flags: &FlagConfig{
+				WebSystemdSocket: OfBool(true),
+				WebConfigFile:    OfString(""),
+			},
+			want: nil,
+		},
+		// Regression test: a non-nil but false systemd socket flag used to
+		// satisfy checkFlags, so ListenAndServe went on to dereference the nil
+		// WebListenAddresses and panicked.
+		{
+			name: "systemd socket activation disabled with nil listen addresses",
+			flags: &FlagConfig{
+				WebSystemdSocket: OfBool(false),
+				WebConfigFile:    OfString(""),
+			},
+			want: ErrNoListeners,
+		},
+		{
+			name: "systemd socket activation disabled with empty listen addresses",
+			flags: &FlagConfig{
+				WebListenAddresses: &[]string{},
+				WebSystemdSocket:   OfBool(false),
+				WebConfigFile:      OfString(""),
+			},
+			want: ErrNoListeners,
+		},
+		{
+			name: "no listeners configured at all",
+			flags: &FlagConfig{
+				WebConfigFile: OfString(""),
+			},
+			want: ErrNoListeners,
+		},
+	} {
+		t.Run(tc.name, func(t *testing.T) {
+			if err := tc.flags.checkFlags(); !errors.Is(err, tc.want) {
+				t.Fatalf("checkFlags() = %v, want %v", err, tc.want)
+			}
+		})
+	}
+}
+
+// TestListenAndServeNoListeners checks that ListenAndServe reports a missing
+// listener configuration instead of panicking on a nil WebListenAddresses.
+func TestListenAndServeNoListeners(t *testing.T) {
+	flags := &FlagConfig{
+		WebSystemdSocket: OfBool(false),
+		WebConfigFile:    OfString(""),
+	}
+	if err := ListenAndServe(nil, flags, testlogger); !errors.Is(err, ErrNoListeners) {
+		t.Fatalf("ListenAndServe() = %v, want %v", err, ErrNoListeners)
+	}
+}
diff --git a/web/tls_config.go b/web/tls_config.go
index b40be6bb..2e8a5208 100644
--- a/web/tls_config.go
+++ b/web/tls_config.go
@@ -84,8 +84,15 @@ func (c *FlagConfig) checkFlags() error {
 	if c.WebConfigFile == nil {
 		return ErrMissingFlag
 	}
-	if c.WebSystemdSocket == nil && (c.WebListenAddresses == nil || len(*c.WebListenAddresses) == 0) {
-		return ErrNoListeners
+	// Listen addresses are only optional when systemd socket activation is
+	// actually enabled. Checking that WebSystemdSocket is non-nil is not
+	// enough: kingpinflag.AddFlags always hands out a non-nil pointer, so a
+	// nil-but-false flag would otherwise pass validation and then panic on the
+	// *flags.WebListenAddresses dereference in ListenAndServe.
+	if c.WebSystemdSocket == nil || !*c.WebSystemdSocket {
+		if c.WebListenAddresses == nil || len(*c.WebListenAddresses) == 0 {
+			return ErrNoListeners
+		}
 	}
 	return nil
 }