Denial of Service (crash) via input validation panic in ResourceSlice validation

Proof-of-concept (requires a cluster with the DRA ResourceSlice CRD installed and OpenAPI validation enabled): 1) Create a ResourceSlice resource with a device capacity that uses a validRange.step of 0 (or a negative value). The API server should reject the object with a validation error rather than crash. Example YAML (structure corresponds to the ResourceSlice CRD used by the DRA feature; adjust field names if your CRD uses different aliases): apiVersion: resource.k8s.io/v1alpha1 kind: ResourceSlice metadata: name: test-rslice-step0 spec: devices: - name: device-cap1 capacity: cap: value: "1Gi" requestPolicy: default: "10Mi" validRange: min: "1Mi" max: "100Mi" step: "0" # intentionally invalid: must be > 0 # Expected outcome: # The API server should return a 400 Bad Request with a message similar to: # must be greater than zero # and will not crash. This demonstrates the patch handling invalid steps gracefully.

diff --git a/pkg/apis/resource/validation/validation.go b/pkg/apis/resource/validation/validation.go index 0706817a328f1..8f28bf073d7f0 100644 --- a/pkg/apis/resource/validation/validation.go +++ b/pkg/apis/resource/validation/validation.go @@ -1175,14 +1175,18 @@ func validateRequestPolicyRange(defaultValue apiresource.Quantity, maxCapacity a } } if valueRange.Step != nil { - added := valueRange.Min.DeepCopy() - added.Add(*valueRange.Step) - if added.Cmp(maxCapacity) > 0 { - allErrs = append(allErrs, field.Invalid(fldPath.Child("step"), valueRange.Step.String(), fmt.Sprintf("one step %s is larger than capacity value: %s", added.String(), maxCapacity.String()))) - } - allErrs = append(allErrs, validateRequestPolicyRangeStep(defaultValue, *valueRange.Min, *valueRange.Step, fldPath.Child("step"))...) - if valueRange.Max != nil { - allErrs = append(allErrs, validateRequestPolicyRangeStep(*valueRange.Max, *valueRange.Min, *valueRange.Step, fldPath.Child("step"))...) + if valueRange.Step.Sign() <= 0 { + allErrs = append(allErrs, field.Invalid(fldPath.Child("step"), valueRange.Step.String(), "must be greater than zero")) + } else { + added := valueRange.Min.DeepCopy() + added.Add(*valueRange.Step) + if added.Cmp(maxCapacity) > 0 { + allErrs = append(allErrs, field.Invalid(fldPath.Child("step"), valueRange.Step.String(), fmt.Sprintf("one step %s is larger than capacity value: %s", added.String(), maxCapacity.String()))) + } + allErrs = append(allErrs, validateRequestPolicyRangeStep(defaultValue, *valueRange.Min, *valueRange.Step, fldPath.Child("step"))...) + if valueRange.Max != nil { + allErrs = append(allErrs, validateRequestPolicyRangeStep(*valueRange.Max, *valueRange.Min, *valueRange.Step, fldPath.Child("step"))...) + } } } return allErrs diff --git a/pkg/apis/resource/validation/validation_resourceslice_test.go b/pkg/apis/resource/validation/validation_resourceslice_test.go index 8e763c0bbe6ac..7570caeb2e204 100644 --- a/pkg/apis/resource/validation/validation_resourceslice_test.go +++ b/pkg/apis/resource/validation/validation_resourceslice_test.go @@ -834,6 +834,56 @@ func TestValidateResourceSlice(t *testing.T) { }(), consumableCapacityFeatureGate: true, }, + "invalid-request-policy-zero-step": { + wantFailures: field.ErrorList{ + field.Invalid(field.NewPath("spec", "devices").Index(1).Child("capacity").Key("cap").Child("requestPolicy").Child("validRange", "step"), "0", "must be greater than zero"), + }, + slice: func() *resourceapi.ResourceSlice { + slice := testResourceSlice(goodName, goodName, goodName, 2) + slice.Spec.Devices[1].AllowMultipleAllocations = ptr.To(true) + capacity := resourceapi.DeviceCapacity{ + Value: resource.MustParse("1Gi"), + RequestPolicy: &resourceapi.CapacityRequestPolicy{ + Default: ptr.To(resource.MustParse("10Mi")), + ValidRange: &resourceapi.CapacityRequestPolicyRange{ + Min: ptr.To(resource.MustParse("1Mi")), + Max: ptr.To(resource.MustParse("100Mi")), + Step: ptr.To(resource.MustParse("0")), + }, + }, + } + slice.Spec.Devices[1].Capacity = map[resourceapi.QualifiedName]resourceapi.DeviceCapacity{ + "cap": capacity, + } + return slice + }(), + consumableCapacityFeatureGate: true, + }, + "invalid-request-policy-negative-step": { + wantFailures: field.ErrorList{ + field.Invalid(field.NewPath("spec", "devices").Index(1).Child("capacity").Key("cap").Child("requestPolicy").Child("validRange", "step"), "-1Mi", "must be greater than zero"), + }, + slice: func() *resourceapi.ResourceSlice { + slice := testResourceSlice(goodName, goodName, goodName, 2) + slice.Spec.Devices[1].AllowMultipleAllocations = ptr.To(true) + capacity := resourceapi.DeviceCapacity{ + Value: resource.MustParse("1Gi"), + RequestPolicy: &resourceapi.CapacityRequestPolicy{ + Default: ptr.To(resource.MustParse("10Mi")), + ValidRange: &resourceapi.CapacityRequestPolicyRange{ + Min: ptr.To(resource.MustParse("1Mi")), + Max: ptr.To(resource.MustParse("100Mi")), + Step: ptr.To(resource.MustParse("-1Mi")), + }, + }, + } + slice.Spec.Devices[1].Capacity = map[resourceapi.QualifiedName]resourceapi.DeviceCapacity{ + "cap": capacity, + } + return slice + }(), + consumableCapacityFeatureGate: true, + }, "invalid-node-selecor-label-value": { wantFailures: field.ErrorList{field.Invalid(field.NewPath("spec", "nodeSelector", "nodeSelectorTerms").Index(0).Child("matchExpressions").Index(0).Child("values").Index(0), "-1", "a valid label must be an empty string or consist of alphanumeric characters, '-', '_' or '.', and must start and end with an alphanumeric character (e.g. 'MyValue', or 'my_value', or '12345', regex used for validation is '(([A-Za-z0-9][-A-Za-z0-9_.]*)?[A-Za-z0-9])?')")}, slice: func() *resourceapi.ResourceSlice {