Denial of Service (panic/crash via invalid runtime manifest)

PoC (Go) demonstrating the vulnerable behavior prior to this fix: Note: This snippet uses Grafana's internal types in their repo paths. It constructs a deliberately invalid manifest that would previously trigger a panic when loaded at runtime, illustrating a denial-of-service vector. In the fixed version, this call returns an error instead of panicking. package main import ( app "github.com/grafana/grafana/pkg/app" res "github.com/grafana/grafana/pkg/storage/unified/resource" "fmt" ) func main() { // Construct an invalid manifest declaration (as used in tests for regression) invalid := app.Manifest{ManifestData: &app.ManifestData{ Group: "widgets.example.test", Versions: []app.ManifestVersion{{ Name: "v1", Kinds: []app.ManifestVersionKind{{ Kind: "Widget", Plural: "widgets", SearchFields: []app.ManifestVersionKindSearchField{{ Name: "size", Path: "spec.size", Type: "int64", Capabilities: []string{"text"}, }}, }}, }}, }} // This call would panic in vulnerable versions; in this patch it should return an error _, err := res.SearchFieldProviders([]app.Manifest{invalid}) if err != nil { fmt.Println("error:", err) } else { fmt.Println("no error (unexpected in vulnerable code)") } }

diff --git a/pkg/storage/unified/resource/search_field.go b/pkg/storage/unified/resource/search_field.go index a0073ef248f6..5b408f862818 100644 --- a/pkg/storage/unified/resource/search_field.go +++ b/pkg/storage/unified/resource/search_field.go @@ -259,11 +259,19 @@ type mapProvider struct { // maps. Both arguments may be nil. The provider takes ownership of the maps; // callers must not mutate them after the call. // -// Panics if any SearchFieldDefinition violates validateSearchFieldDefinitions -// (e.g. SearchCapabilitySort declared on a numeric or boolean type). Such a -// declaration is a programmer error in static configuration; the process -// cannot serve correct results with an invalid mapping. +// Panics on an invalid definition: in static build-time config that is a +// programmer error. Runtime callers use newMapProvider instead. func NewMapProvider(fields map[schema.GroupVersionResource][]SearchFieldDefinition, preferredVersions map[schema.GroupResource]string) SearchFieldsProvider { + p, err := newMapProvider(fields, preferredVersions) + if err != nil { + panic(err.Error()) + } + return p +} + +// newMapProvider is NewMapProvider's error-returning core, so a caller +// ingesting definitions at runtime can reject a bad set instead of crashing. +func newMapProvider(fields map[schema.GroupVersionResource][]SearchFieldDefinition, preferredVersions map[schema.GroupResource]string) (SearchFieldsProvider, error) { if fields == nil { fields = map[schema.GroupVersionResource][]SearchFieldDefinition{} } @@ -272,16 +280,16 @@ func NewMapProvider(fields map[schema.GroupVersionResource][]SearchFieldDefiniti } for gvr, sfds := range fields { if err := validateSearchFieldDefinitions(sfds); err != nil { - panic("invalid SearchFieldDefinitions for " + gvr.String() + ": " + err.Error()) + return nil, fmt.Errorf("invalid SearchFieldDefinitions for %s: %w", gvr.String(), err) } } if err := validateCrossVersionConsistency(fields); err != nil { - panic("inconsistent SearchFieldDefinitions across versions: " + err.Error()) + return nil, fmt.Errorf("inconsistent SearchFieldDefinitions across versions: %w", err) } return &mapProvider{ fields: fields, preferredVersion: preferredVersions, - } + }, nil } // mappingAttributes is the bleve-mapping-affecting projection of a diff --git a/pkg/storage/unified/resource/search_field_manifest.go b/pkg/storage/unified/resource/search_field_manifest.go index b2463c52f732..59449a929d64 100644 --- a/pkg/storage/unified/resource/search_field_manifest.go +++ b/pkg/storage/unified/resource/search_field_manifest.go @@ -15,7 +15,20 @@ import ( // // CopyFromStandard has no manifest counterpart and is never set from a // manifest. A kind that needs it keeps a builder-side provider instead. +// +// Panics on an invalid declaration, like NewMapProvider. Runtime callers use +// newManifestBackedProvider instead. func NewManifestBackedProvider(manifests []app.Manifest) SearchFieldsProvider { + p, err := newManifestBackedProvider(manifests) + if err != nil { + panic(err.Error()) + } + return p +} + +// newManifestBackedProvider is NewManifestBackedProvider's error-returning +// core, so a runtime manifest source can reject a bad set instead of crashing. +func newManifestBackedProvider(manifests []app.Manifest) (SearchFieldsProvider, error) { fields := map[schema.GroupVersionResource][]SearchFieldDefinition{} preferred := map[schema.GroupResource]string{} @@ -47,7 +60,7 @@ func NewManifestBackedProvider(manifests []app.Manifest) SearchFieldsProvider { } } } - return NewMapProvider(fields, preferred) + return newMapProvider(fields, preferred) } // manifestResourceName returns the lower-cased resource (plural) name for a @@ -109,18 +122,21 @@ func manifestDeclaredKindKeys(manifests []app.Manifest) map[LowerGroupResource]b // drives bleve mappings. Every kind that declares search fields in its manifest // is mapped to a single manifest-backed provider; each entry queries it for its // own (group, resource). -func SearchFieldProviders(manifests []app.Manifest) map[LowerGroupResource]SearchFieldsProvider { +func SearchFieldProviders(manifests []app.Manifest) (map[LowerGroupResource]SearchFieldsProvider, error) { declared := manifestDeclaredKindKeys(manifests) out := make(map[LowerGroupResource]SearchFieldsProvider, len(declared)) if len(declared) == 0 { - return out + return out, nil } // A single manifest-backed provider covers every declared kind; each map // entry queries it for its own (group, resource). - manifestProvider := NewManifestBackedProvider(manifests) + manifestProvider, err := newManifestBackedProvider(manifests) + if err != nil { + return nil, err + } for key := range declared { out[key] = manifestProvider } - return out + return out, nil } diff --git a/pkg/storage/unified/resource/search_field_manifest_test.go b/pkg/storage/unified/resource/search_field_manifest_test.go index 93409a2bb0ed..fdb6f2255d54 100644 --- a/pkg/storage/unified/resource/search_field_manifest_test.go +++ b/pkg/storage/unified/resource/search_field_manifest_test.go @@ -94,7 +94,8 @@ func TestNewManifestBackedProvider_DefaultsPluralAndPreferredVersion(t *testing. } func TestSearchFieldProviders_MapsDeclaredKinds(t *testing.T) { - got := SearchFieldProviders([]app.Manifest{manifestWithSearchFields()}) + got, err := SearchFieldProviders([]app.Manifest{manifestWithSearchFields()}) + require.NoError(t, err) // The one kind that declares search fields is present and provider-backed. require.Len(t, got, 1) @@ -116,6 +117,29 @@ func TestSearchFieldProviders_NoManifestDeclarationsIsEmpty(t *testing.T) { }}, }} - got := SearchFieldProviders([]app.Manifest{manifestNoFields}) + got, err := SearchFieldProviders([]app.Manifest{manifestNoFields}) + require.NoError(t, err) assert.Empty(t, got) } + +func TestSearchFieldProviders_InvalidManifestReturnsError(t *testing.T) { + // A runtime manifest source can carry an invalid declaration; the path must + // surface it as an error rather than panicking. + invalid := app.Manifest{ManifestData: &app.ManifestData{ + Group: "widgets.example.test", + Versions: []app.ManifestVersion{{ + Name: "v1", + Kinds: []app.ManifestVersionKind{{ + Kind: "Widget", + Plural: "widgets", + SearchFields: []app.ManifestVersionKindSearchField{ + {Name: "size", Path: "spec.size", Type: "int64", Capabilities: []string{"text"}}, + }, + }}, + }}, + }} + + got, err := SearchFieldProviders([]app.Manifest{invalid}) + require.Error(t, err) + assert.Nil(t, got) +} diff --git a/pkg/storage/unified/search/options.go b/pkg/storage/unified/search/options.go index a268b12a3e72..fd8d7f82ed58 100644 --- a/pkg/storage/unified/search/options.go +++ b/pkg/storage/unified/search/options.go @@ -99,7 +99,10 @@ func NewSearchOptions( searchFieldsHashes = resource.SearchFieldsHashesForBuilders(builders) // Search fields come from the app manifests: every in-tree kind that // has custom search fields declares them in its CUE manifest. - searchFieldsProviders = resource.SearchFieldProviders(resource.AppManifests()) + searchFieldsProviders, err = resource.SearchFieldProviders(resource.AppManifests()) + if err != nil { + return resource.SearchOptions{}, err + } } bleve, err := NewBleveBackend(BleveOptions{