GHSA-f94q-w3w8-cj67 Capsule: hostnameRegexHandler.OnUpdate validates stale (old) Tenant regex, allowing invalid AllowedHostnames regex to bypass webhook validation

• Go github.com/projectcapsule/capsule: >= 0.13.0, < 0.13.7 → fixed in 0.13.7

Summary A parameter order bug in `internal/webhook/tenant/validation/hostname_regex.go` causes the `hostnameRegexHandler.OnUpdate` webhook to validate the old Tenant object's `AllowedHostnames.Regex` instead of the new one being submitted. This allows an invalid (malformed) regex to bypass admission validation and be persisted to etcd, causing a Denial of Service for all Ingress operations within the affected tenant. Details The `TypedHandler[T]` interface defines `OnUpdate` as: ```go // handlers.go OnUpdate(c client.Client, reader client.Reader, obj T, old T, decoder admission.Decoder, recorder events.EventRecorder) Func // ^^^ NEW ^^^ OLD ``` The dispatcher in `handler.go:93` calls: ```go hndl.OnUpdate(c, reader, tnt, old, decoder, recorder) // ^^^ NEW ^^^ OLD ``` However, `hostnameRegexHandler.OnUpdate` in `hostname_regex.go` declares its parameters in reversed order: ```go // hostname_regex.go (BUGGY) func (h *hostnameRegexHandler) OnUpdate( _ client.Client, _ client.Reader, old *capsulev1beta2.Tenant, // ← receives NEW tenant (mislabeled as old) tnt *capsulev1beta2.Tenant, // ← receives OLD tenant (mislabeled as tnt) ... ) handlers.Func { return func(...) *admission.Response { if err := h.validate(tnt, req); err != nil { // ← validates OLD, not NEW return err } return nil } } ``` All 11 other handlers in the same package declare `(tnt, old)` correctly. `hostname_regex.go` is the only one with the swap. As a result, when a Cluster Admin updates `Tenant.Spec.IngressOptions.AllowedHostnames.Regex` to a malformed value, the webhook compiles the previous valid regex and returns `Allow`. The malformed regex is then written to etcd. Subsequently, every Ingress `CREATE` or `UPDATE` in that tenant triggers `validate_hostnames.go:160`: ```go matched, _ = regexp.MatchString(allowedRegex, currentHostname) ``` `regexp.MatchString` with an invalid pattern returns `(false, error)`. The error is silently ignored, `matched` is `false`, and every hostname is rejected — blocking all Ingress operations in the tenant until the Tenant object is manually corrected by an admin. PoC ``` //go:build ignore // Standalone reproducer for hostname_regex.go argument swap bug in Capsule // No external deps - shows the bug logic using only stdlib package main import ( "fmt" "regexp" ) // Simulating the Tenant spec structure type AllowedHostnames struct { Regex string } type IngressOptions struct { AllowedHostnames *AllowedHostnames } type TenantSpec struct { IngressOptions IngressOptions } type Tenant struct { Name string Spec TenantSpec } // ========================================================= // BUGGY implementation (hostname_regex.go as-is) // OnUpdate(_, _, old *Tenant, tnt *Tenant) → validates OLD // ========================================================= func hostnameValidate(tnt *Tenant) error { if tnt.Spec.IngressOptions.AllowedHostnames == nil { return nil } if len(tnt.Spec.IngressOptions.AllowedHostnames.Regex) == 0 { return nil } _, err := regexp.Compile(tnt.Spec.IngressOptions.AllowedHostnames.Regex) if err != nil { return fmt.Errorf("Deny: unable to compile allowedHostnames allowedRegex") } return nil } // Dispatcher calls: OnUpdate(c, reader, newTenant, oldTenant, ...) // Interface says: OnUpdate(c, reader, obj[NEW], old[OLD], ...) // // BUGGY handler receives: (old, tnt) meaning: // 3rd param (labeled "old") = actually NEW // 4th param (labeled "tnt") = actually OLD // Then calls h.validate(tnt) = validates the OLD tenant func buggyOnUpdate(newTenant, oldTenant *Tenant) error { // BUG: parameters are SWAPPED vs the interface contract old := newTenant // dispatcher's "new" arrives as "old" in this function tnt := oldTenant // dispatcher's "old" arrives as "tnt" in this function _ = old // unused in the real code too return hostnameValidate(tnt) // validates OLD, not NEW } // CORRECT implementation (what it should be) func correctOnUpdate(newTenant, oldTenant *Tenant) error { _ = oldTenant return hostnameValidate(newTenant) // validates NEW } // Simulate ingress hostname validation AFTER bad regex is stored func validateIngressHostname(tenant *Tenant, hostname string) bool { if tenant.Spec.IngressOptions.AllowedHostnames == nil { return true } allowedRegex := tenant.Spec.IngressOptions.AllowedHostnames.Regex if len(allowedRegex) == 0 { return true } // This is validate_hostnames.go:160 - error is IGNORED matched, _ := regexp.MatchString(allowedRegex, hostname) return matched } func main() { fmt.Println("=== Capsule Bug Reproducer: hostname_regex.go argument swap ===") fmt.Println() oldTenant := &Tenant{ Name: "demo-tenant", Spec: TenantSpec{ IngressOptions: IngressOptions{ AllowedHostnames: &AllowedHostnames{ Regex: `^[\w.-]+\.example\.com$`, // valid regex }, }, }, } // Attacker (cluster admin) sets an INVALID regex in the new spec newTenant := &Tenant{ Name: "demo-tenant", Spec: TenantSpec{ IngressOptions: IngressOptions{ AllowedHostnames: &AllowedHostnames{ Regex: `[invalid-regex(`, // INVALID regex }, }, }, } fmt.Printf("Old tenant regex: %q (valid)\n", oldTenant.Spec.IngressOptions.AllowedHostnames.Regex) fmt.Printf("New tenant regex: %q (INVALID)\n", newTenant.Spec.IngressOptions.AllowedHostnames.Regex) fmt.Println() // Step 1: Webhook runs OnUpdate fmt.Println("--- Step 1: Webhook OnUpdate ---") err := buggyOnUpdate(newTenant, oldTenant) if err != nil { fmt.Printf("[BUGGY] Webhook DENIES update: %v\n", err) } else { fmt.Println("[BUGGY] Webhook ALLOWS update (validates OLD regex) ← WRONG") } err = correctOnUpdate(newTenant, oldTenant) if err != nil { fmt.Printf("[CORRECT] Webhook DENIES update: %v ← EXPECTED\n", err) } else { fmt.Println("[CORRECT] Webhook ALLOWS update") } // Step 2: Invalid regex now stored in etcd - simulate ingress validation fmt.Println() fmt.Println("--- Step 2: Ingress creation after bad regex stored ---") storedTenant := newTenant // bad regex is now in etcd hostnames := []string{ "app.example.com", "api.example.com", "evil.attacker.com", } for _, h := range hostnames { allowed := validateIngressHostname(storedTenant, h) fmt.Printf(" Ingress hostname %q → allowed=%v", h, allowed) if !allowed { fmt.Print(" ← BLOCKED (DoS: invalid regex causes all hostnames to fail)") } fmt.Println() } fmt.Println() fmt.Println("=== Result ===") fmt.Println("Invalid regex bypasses webhook validation and gets stored.") fmt.Println("All subsequent Ingress create/update in this tenant are BLOCKED.") fmt.Println("CWE-697: Incorrect Comparison — wrong Tenant object is validated.") } ``` ```go // Simulates the buggy webhook behaviour oldTenant := &Tenant{AllowedRegex: `^[\w-]+\.example\.com$`} // valid newTenant := &Tenant{AllowedRegex: `[invalid-regex(`} // malformed // Buggy OnUpdate: validates oldTenant (valid) → ALLOW // Correct OnUpdate: validates newTenant (invalid) → DENY // After malformed regex is stored, all ingress hostnames are rejected: matched, _ := regexp.MatchString(`[invalid-regex(`, "app.example.com") // matched = false, error ignored → Ingress blocked ``` Fix Swap the parameter names in `hostname_regex.go` to match the interface contract: ```go // BEFORE (buggy) func (h *hostnameRegexHandler) OnUpdate( _ client.Client, _ client.Reader, old *capsulev1beta2.Tenant, tnt *capsulev1beta2.Tenant, ... // AFTER (fixed) func (h *hostnameRegexHandler) OnUpdate( _ client.Client, _ client.Reader, tnt *capsulev1beta2.Tenant, old *capsulev1beta2.Tenant, ... ``` Impact A Cluster Admin (or a compromised admin account) can — intentionally or via a typo — set a malformed `AllowedHostnames.Regex` on any Tenant. The webhook silently a…