Memory safety / panic (crash) vulnerability in KV parsing

Go PoC code snippet: package main import ( "fmt" "strings" kv "github.com/grafana/grafana/pkg/storage/unified/resource/kv" ) func main() { // Craft a key with rvMeta that has only a single tilde-separated segment (would panic pre-fix) parts := strings.Split("g/res/n/99", "/") dk, rvParts, err := kv.ParseDataKeyParts(parts) fmt.Printf("dk=%+v rvParts=%v err=%v\n", dk, rvParts, err) }

diff --git a/pkg/storage/unified/resource/kv/datastoretypes.go b/pkg/storage/unified/resource/kv/datastoretypes.go index 905c2d7cd42f..ed9651ee0a4d 100644 --- a/pkg/storage/unified/resource/kv/datastoretypes.go +++ b/pkg/storage/unified/resource/kv/datastoretypes.go @@ -68,6 +68,9 @@ func ParseDataKeyParts(parts []string) (DataKey, []string, error) { return DataKey{}, nil, fmt.Errorf("invalid key: expected 4 or 5 parts, got %d", len(parts)) } rvParts := strings.Split(rvMeta, "~") + if len(rvParts) < 3 { + return DataKey{}, nil, fmt.Errorf("invalid resource version metadata: expected at least 3 parts, got %d", len(rvParts)) + } rv, err := strconv.ParseInt(rvParts[0], 10, 64) if err != nil { return DataKey{}, nil, fmt.Errorf("invalid resource version '%s': %w", rvParts[0], err) diff --git a/pkg/storage/unified/resource/kv/datastoretypes_test.go b/pkg/storage/unified/resource/kv/datastoretypes_test.go new file mode 100644 index 000000000000..139ae0f6bcfe --- /dev/null +++ b/pkg/storage/unified/resource/kv/datastoretypes_test.go @@ -0,0 +1,102 @@ +package kv + +import ( + "strings" + "testing" + + "github.com/stretchr/testify/require" +) + +func TestParseDataKeyParts_ResourceVersionMetadata(t *testing.T) { + t.Parallel() + + t.Run("cluster-scoped valid three-part rvMeta", func(t *testing.T) { + t.Parallel() + parts := strings.Split("g/res/n/42~updated~fld", "/") + dk, rvParts, err := ParseDataKeyParts(parts) + require.NoError(t, err) + require.Equal(t, "g", dk.Group) + require.Equal(t, "res", dk.Resource) + require.Equal(t, "n", dk.Name) + require.Equal(t, int64(42), dk.ResourceVersion) + require.Equal(t, DataActionUpdated, dk.Action) + require.Equal(t, "fld", dk.Folder) + require.Equal(t, []string{"42", "updated", "fld"}, rvParts) + }) + + t.Run("namespaced valid three-part rvMeta", func(t *testing.T) { + t.Parallel() + parts := strings.Split("g/res/ns/n/1~created~", "/") + dk, rvParts, err := ParseDataKeyParts(parts) + require.NoError(t, err) + require.Equal(t, "ns", dk.Namespace) + require.Equal(t, "n", dk.Name) + require.Equal(t, int64(1), dk.ResourceVersion) + require.Equal(t, DataActionCreated, dk.Action) + require.Equal(t, "", dk.Folder) + require.Equal(t, []string{"1", "created", ""}, rvParts) + }) + + t.Run("rvMeta single segment returns clear error", func(t *testing.T) { + t.Parallel() + // Previously this could panic indexing rvParts[1]. + parts := strings.Split("g/res/n/99", "/") + _, _, err := ParseDataKeyParts(parts) + require.Error(t, err) + require.Contains(t, err.Error(), "invalid resource version metadata") + require.Contains(t, err.Error(), "expected at least 3 parts, got 1") + }) + + t.Run("rvMeta two segments returns clear error", func(t *testing.T) { + t.Parallel() + parts := strings.Split("g/res/n/1~created", "/") + _, _, err := ParseDataKeyParts(parts) + require.Error(t, err) + require.Contains(t, err.Error(), "expected at least 3 parts, got 2") + }) + + t.Run("rvMeta four tilde segments parses base fields and preserves full rvParts", func(t *testing.T) { + t.Parallel() + // Fourth segment is consumed by ParseKeyWithGUID as GUID, not as folder. + parts := strings.Split("g/res/n/1~created~f~guid-xyz", "/") + dk, rvParts, err := ParseDataKeyParts(parts) + require.NoError(t, err) + require.Equal(t, int64(1), dk.ResourceVersion) + require.Equal(t, DataActionCreated, dk.Action) + require.Equal(t, "f", dk.Folder) + require.Equal(t, []string{"1", "created", "f", "guid-xyz"}, rvParts) + }) +} + +func TestParseKeyWithGUID(t *testing.T) { + t.Parallel() + + t.Run("round trip StringWithGUID", func(t *testing.T) { + t.Parallel() + // Namespaced key: group/resource/namespace/name/rv~action~folder~guid (5 slash segments). + key := "apps/deployments/default/nginx/5~updated~fld~guid-abc" + dk, err := ParseKeyWithGUID(key) + require.NoError(t, err) + require.Equal(t, "guid-abc", dk.GUID) + require.Equal(t, int64(5), dk.ResourceVersion) + require.Equal(t, DataActionUpdated, dk.Action) + require.Equal(t, "fld", dk.Folder) + require.Equal(t, key, dk.StringWithGUID()) + }) + + t.Run("rejects key with only three tilde parts in rvMeta", func(t *testing.T) { + t.Parallel() + // ParseDataKeyParts succeeds but GUID requires fourth segment. + _, err := ParseKeyWithGUID("g/res/n/1~created~f") + require.Error(t, err) + require.Contains(t, err.Error(), "invalid key metadata") + require.Contains(t, err.Error(), "expected 4 tilde-separated parts, got 3") + }) + + t.Run("invalid rvMeta surfaces from ParseDataKeyParts", func(t *testing.T) { + t.Parallel() + _, err := ParseKeyWithGUID("g/res/n/badrv") + require.Error(t, err) + require.Contains(t, err.Error(), "invalid resource version metadata") + }) +}