From a48c9587881b447ad8e4d987ba4f3839d38f5396 Mon Sep 17 00:00:00 2001
From: Mark Gritter <mgritter@hashicorp.com>
Date: Wed, 2 Sep 2020 17:43:44 -0500
Subject: [PATCH] Fix crash when KV store has a zero-length key. (#9881)

---
 CHANGELOG.md               |  1 +
 vault/core_metrics.go      |  2 +-
 vault/core_metrics_test.go | 57 ++++++++++++++++++++++++++++++++++++++
 3 files changed, 59 insertions(+), 1 deletion(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 4fab070a0a2b..e26ebf410cd3 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -10,6 +10,7 @@ BUG FIXES:
 * core: Handle a trailing slash in the API address used for enabling replication
 * core: Fix resource leak in plugin API (plugin-dependent, not all plugins impacted) [[GH-9557](https://github.com/hashicorp/vault/pull/9557)]
 * core: Fix race involved in enabling certain features via a license change
+* core: Fix crash when metrics collection encounters zero-length keys in KV store [[GH-9811](https://github.com/hashicorp/vault/pull/9881)]
 * secrets/aws: Fix possible issue creating access keys when using Performance Standbys  [[GH-9606](https://github.com/hashicorp/vault/pull/9606)]
 * secrets/database: Fix handling of TLS options in mongodb connection strings [[GH-9519](https://github.com/hashicorp/vault/pull/9519)]
 * secrets/gcp: Ensure that the IAM policy version is appropriately set after a roleset's bindings have changed. [[GH-93](https://github.com/hashicorp/vault-plugin-secrets-gcp/pull/93)]
diff --git a/vault/core_metrics.go b/vault/core_metrics.go
index 4ac2656d11d9..c729bc2a3cf4 100644
--- a/vault/core_metrics.go
+++ b/vault/core_metrics.go
@@ -294,7 +294,7 @@ func (c *Core) walkKvMountSecrets(ctx context.Context, m *kvMount) {
 			return
 		}
 		for _, path := range keys {
-			if path[len(path)-1] == '/' {
+			if len(path) > 0 && path[len(path)-1] == '/' {
 				subdirectories = append(subdirectories, currentDirectory+path)
 			} else {
 				m.NumSecrets += 1
diff --git a/vault/core_metrics_test.go b/vault/core_metrics_test.go
index ffee3b3ad5f1..85a161aaa42a 100644
--- a/vault/core_metrics_test.go
+++ b/vault/core_metrics_test.go
@@ -132,6 +132,63 @@ func TestCoreMetrics_KvSecretGauge(t *testing.T) {
 	}
 }
 
+func TestCoreMetrics_KvSecretGauge_BadPath(t *testing.T) {
+	// Use the real KV implementation instead of Passthrough
+	AddTestLogicalBackend("kv", logicalKv.Factory)
+	// Clean up for the next test.
+	defer func() {
+		delete(testLogicalBackends, "kv")
+	}()
+	core, _, _ := TestCoreUnsealed(t)
+
+	me := &MountEntry{
+		Table:   mountTableType,
+		Path:    sanitizePath("kv1"),
+		Type:    "kv",
+		Options: map[string]string{"version": "1"},
+	}
+	ctx := namespace.RootContext(nil)
+	err := core.mount(ctx, me)
+	if err != nil {
+		t.Fatalf("mount error: %v", err)
+	}
+
+	// I don't think there's any remaining way to create a zero-length
+	// key via the API, so we'll fake it by talking to the storage layer directly.
+	fake_entry := &logical.StorageEntry{
+		Key:   "logical/" + me.UUID + "/foo/",
+		Value: []byte{1},
+	}
+	err = core.barrier.Put(ctx, fake_entry)
+	if err != nil {
+		t.Fatalf("put error: %v", err)
+	}
+
+	values, err := core.kvSecretGaugeCollector(ctx)
+	if err != nil {
+		t.Fatalf("collector error: %v", err)
+	}
+	t.Logf("Values: %v", values)
+	found := false
+	var count float32 = -1
+	for _, glv := range values {
+		for _, l := range glv.Labels {
+			if l.Name == "mount_point" && l.Value == "kv1/" {
+				found = true
+				count = glv.Value
+				break
+			}
+		}
+	}
+	if found {
+		if count != 1.0 {
+			t.Errorf("bad secret count for kv1/")
+		}
+	} else {
+		t.Errorf("no secret count for kv1/")
+	}
+}
+
 func TestCoreMetrics_KvSecretGaugeError(t *testing.T) {
 	core := TestCore(t)