nil pointer dereference in RetriesLimitReached panics controller when rollbackRetriesLimit is omitted #178

What happened?

utils.RetriesLimitReached dereferences its rollbackRetriesLimit *int32 argument without a nil check. The field is declared omitempty in the CRD spec, so it is nil whenever a user creates a DisposableRequest without setting rollbackRetriesLimit. When such a request enters any error state (status.error != ""), the controller calls RetriesLimitReached(cr.Status.Failed, cr.Spec.ForProvider.RollbackRetriesLimit) and panics immediately.

Expected: RetriesLimitReached should guard against a nil pointer just like the companion RollBackEnabled helper does, or the call sites should call RollBackEnabled first (the pattern already used in deployaction.go:34).

Actual: the controller goroutine panics with runtime error: invalid memory address or nil pointer dereference, the reconciliation for that resource is interrupted on every retry, and the error is surfaced as a recovered panic by controller-runtime.

How can we reproduce it?

Minimal Go reproducer (verbatim copy of the function):

package main

import "fmt"

func RetriesLimitReached(statusFailed int32, rollbackRetriesLimit *int32) bool {
    return statusFailed >= *rollbackRetriesLimit
}

func main() {
    defer func() {
        if r := recover(); r != nil {
            fmt.Println("panic:", r)
        }
    }()
    RetriesLimitReached(1, nil)
}

$ go run .
panic: runtime error: invalid memory address or nil pointer dereference

Trigger via a Kubernetes manifest: apply a DisposableRequest (cluster or namespaced) that:

1. omits the rollbackRetriesLimit field (valid, it is omitempty)
2. targets a URL that returns an error or a non-matching expected response

Once status.error is set by the managed reconciler, the errorConditionReconciler wrapper calls RetriesLimitReached with a nil pointer on every subsequent reconcile.

Vulnerable function (internal/utils/retry.go:23-25):

// RetriesLimitReached determines if the rollback retries limit has been reached.
func RetriesLimitReached(statusFailed int32, rollbackRetriesLimit *int32) bool {
    return statusFailed >= *rollbackRetriesLimit  // no nil check before deref
}

The companion helper correctly checks for nil before use, but RetriesLimitReached does not:

// RollBackEnabled determines if the rollback retries limit is enabled.
func RollBackEnabled(rollbackRetriesLimit *int32) bool {
    return rollbackRetriesLimit != nil
}

Call sites that pass nil (no prior nil guard):

internal/controller/cluster/disposablerequest/disposablerequest.go:78
internal/controller/namespaced/disposablerequest/disposablerequest.go:80

Both pass cr.Spec.ForProvider.RollbackRetriesLimit directly, which is nil when the field is omitted from the CR manifest.

Call site that is correctly guarded (internal/service/disposablerequest/deployaction.go:34):

if utils.RollBackEnabled(rollbackPolicy.GetRollbackRetriesLimit()) &&
    utils.RetriesLimitReached(status.GetFailed(), rollbackPolicy.GetRollbackRetriesLimit()) {

Suggested fix: add an early return inside RetriesLimitReached:

func RetriesLimitReached(statusFailed int32, rollbackRetriesLimit *int32) bool {
    if rollbackRetriesLimit == nil {
        return false
    }
    return statusFailed >= *rollbackRetriesLimit
}

What environment did it happen in?

Crossplane version: provider-http v1.0.14 (commit 3bf6f9d)

Discovery method

This bug was found using Zorya, a concolic execution engine for Go binaries. Zorya ran symbolic exploration on a standalone binary wrapping RetriesLimitReached and had Z3 produce a satisfying assignment within 47 seconds.

The exact command used:

zorya zorya_retries_limit_real \
  --mode function 0x4b7200 \
  --thread-scheduling main-only \
  --lang go \
  --compiler gc \
  --arg "02000000 03000000" \
  --negate-path-exploration

--arg "02000000 03000000" provides concrete seed values (statusFailed=2, *rollbackRetriesLimit=3, both as little-endian hex) from which Zorya initialises symbolic variables and explores diverging paths. Z3 witness: rollbackRetriesLimit = 0x0 (nil pointer), confirmed natively with go run.