fix: nil ptr deref

Signed-off-by: Jason Cameron <jason.cameron@stanwith.me>

docs/docs/CHANGELOG.md

@@ -11,8 +11,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ## [Unreleased]
 
+- fix: prevent nil pointer panic in challenge validation when threshold rules match during PassChallenge (#1463)
+
 <!-- This changes the project to: -->
-- Fix CEL internal errors when iterating `headers`/`query` map wrappers by implementing map iterators for `HTTPHeaders` and `URLValues` ([#1465](https://github.com/TecharoHQ/anubis/pull/1465)).
 
 ## v1.25.0: Necron
 

lib/anubis.go

@@ -106,6 +106,13 @@ func (s *Server) issueChallenge(ctx context.Context, r *http.Request, lg *slog.L
 		//return nil, errors.New("[unexpected] this codepath should be impossible, asked to issue a challenge for a non-challenge rule")
 	}
 
+	if rule.Challenge == nil {
+		rule.Challenge = &config.ChallengeRules{
+			Difficulty: s.policy.DefaultDifficulty,
+			Algorithm:  config.DefaultAlgorithm,
+		}
+	}
+
 	id, err := uuid.NewV7()
 	if err != nil {
 		return nil, err
@@ -491,7 +498,11 @@ func (s *Server) PassChallenge(w http.ResponseWriter, r *http.Request) {
 	chall, err := s.getChallenge(r)
 	if err != nil {
 		lg.Error("getChallenge failed", "err", err)
-		s.respondWithError(w, r, fmt.Sprintf("%s: %s", localizer.T("internal_server_error"), rule.Challenge.Algorithm), makeCode(err))
+		algorithm := "unknown"
+		if rule.Challenge != nil {
+			algorithm = rule.Challenge.Algorithm
+		}
+		s.respondWithError(w, r, fmt.Sprintf("%s: %s", localizer.T("internal_server_error"), algorithm), makeCode(err))
 		return
 	}
 
@@ -638,8 +649,16 @@ func (s *Server) check(r *http.Request, lg *slog.Logger) (policy.CheckResult, *p
 		}
 
 		if matches {
+			challRules := t.Challenge
+			if challRules == nil {
+				// Non-CHALLENGE thresholds (ALLOW/DENY) don't have challenge config.
+				// Use an empty struct so hydrateChallengeRule can fill from stored
+				// challenge data during validation, rather than baking in defaults
+				// that could mismatch the difficulty the client actually solved for.
+				challRules = &config.ChallengeRules{}
+			}
 			return cr("threshold/"+t.Name, t.Action, weight), &policy.Bot{
-				Challenge: t.Challenge,
+				Challenge: challRules,
 				Rules:     &checker.List{},
 			}, nil
 		}

lib/challenge/error.go

@@ -10,6 +10,7 @@ var (
 	ErrFailed        = errors.New("challenge: user failed challenge")
 	ErrMissingField  = errors.New("challenge: missing field")
 	ErrInvalidFormat = errors.New("challenge: field has invalid format")
+	ErrInvalidInput  = errors.New("challenge: input is nil or missing required fields")
 )
 
 func NewError(verb, publicReason string, privateReason error) *Error {

lib/challenge/interface.go

@@ -1,6 +1,7 @@
 package challenge
 
 import (
+	"fmt"
 	"log/slog"
 	"net/http"
 	"sort"
@@ -50,12 +51,44 @@ type IssueInput struct {
 	Store     store.Interface
 }
 
+func (in *IssueInput) Valid() error {
+	if in == nil {
+		return fmt.Errorf("%w: IssueInput is nil", ErrInvalidInput)
+	}
+	if in.Rule == nil {
+		return fmt.Errorf("%w: Rule is nil", ErrInvalidInput)
+	}
+	if in.Rule.Challenge == nil {
+		return fmt.Errorf("%w: Rule.Challenge is nil", ErrInvalidInput)
+	}
+	if in.Challenge == nil {
+		return fmt.Errorf("%w: Challenge is nil", ErrInvalidInput)
+	}
+	return nil
+}
+
 type ValidateInput struct {
 	Rule      *policy.Bot
 	Challenge *Challenge
 	Store     store.Interface
 }
 
+func (in *ValidateInput) Valid() error {
+	if in == nil {
+		return fmt.Errorf("%w: ValidateInput is nil", ErrInvalidInput)
+	}
+	if in.Rule == nil {
+		return fmt.Errorf("%w: Rule is nil", ErrInvalidInput)
+	}
+	if in.Rule.Challenge == nil {
+		return fmt.Errorf("%w: Rule.Challenge is nil", ErrInvalidInput)
+	}
+	if in.Challenge == nil {
+		return fmt.Errorf("%w: Challenge is nil", ErrInvalidInput)
+	}
+	return nil
+}
+
 type Impl interface {
 	// Setup registers any additional routes with the Impl for assets or API routes.
 	Setup(mux *http.ServeMux)

lib/challenge/metarefresh/metarefresh.go

@@ -24,6 +24,10 @@ type Impl struct{}
 func (i *Impl) Setup(mux *http.ServeMux) {}
 
 func (i *Impl) Issue(w http.ResponseWriter, r *http.Request, lg *slog.Logger, in *challenge.IssueInput) (templ.Component, error) {
+	if err := in.Valid(); err != nil {
+		return nil, err
+	}
+
 	u, err := r.URL.Parse(anubis.BasePrefix + "/.within.website/x/cmd/anubis/api/pass-challenge")
 	if err != nil {
 		return nil, fmt.Errorf("can't render page: %w", err)
@@ -49,6 +53,10 @@ func (i *Impl) Issue(w http.ResponseWriter, r *http.Request, lg *slog.Logger, in
 }
 
 func (i *Impl) Validate(r *http.Request, lg *slog.Logger, in *challenge.ValidateInput) error {
+	if err := in.Valid(); err != nil {
+		return challenge.NewError("validate", "invalid input", err)
+	}
+
 	wantTime := in.Challenge.IssuedAt.Add(time.Duration(in.Rule.Challenge.Difficulty) * 800 * time.Millisecond)
 
 	if time.Now().Before(wantTime) {

lib/challenge/preact/preact.go

@@ -39,6 +39,10 @@ type impl struct{}
 func (i *impl) Setup(mux *http.ServeMux) {}
 
 func (i *impl) Issue(w http.ResponseWriter, r *http.Request, lg *slog.Logger, in *challenge.IssueInput) (templ.Component, error) {
+	if err := in.Valid(); err != nil {
+		return nil, err
+	}
+
 	u, err := r.URL.Parse(anubis.BasePrefix + "/.within.website/x/cmd/anubis/api/pass-challenge")
 	if err != nil {
 		return nil, fmt.Errorf("can't render page: %w", err)
@@ -57,6 +61,10 @@ func (i *impl) Issue(w http.ResponseWriter, r *http.Request, lg *slog.Logger, in
 }
 
 func (i *impl) Validate(r *http.Request, lg *slog.Logger, in *challenge.ValidateInput) error {
+	if err := in.Valid(); err != nil {
+		return challenge.NewError("validate", "invalid input", err)
+	}
+
 	wantTime := in.Challenge.IssuedAt.Add(time.Duration(in.Rule.Challenge.Difficulty) * 80 * time.Millisecond)
 
 	if time.Now().Before(wantTime) {

lib/challenge/proofofwork/proofofwork.go

@@ -33,6 +33,10 @@ func (i *Impl) Issue(w http.ResponseWriter, r *http.Request, lg *slog.Logger, in
 }
 
 func (i *Impl) Validate(r *http.Request, lg *slog.Logger, in *chall.ValidateInput) error {
+	if err := in.Valid(); err != nil {
+		return chall.NewError("validate", "invalid input", err)
+	}
+
 	rule := in.Rule
 	challenge := in.Challenge.RandomData
 

lib/challenge/proofofwork/proofofwork_test.go

@@ -30,6 +30,62 @@ func mkRequest(t *testing.T, values map[string]string) *http.Request {
 	return req
 }
 
+// TestValidateNilRuleChallenge reproduces the panic from
+// https://github.com/TecharoHQ/anubis/issues/1463
+//
+// When a threshold rule matches during PassChallenge, check() can return
+// a policy.Bot with Challenge == nil. After hydrateChallengeRule fails to
+// run (or the error path hits before it), Validate dereferences
+// rule.Challenge.Difficulty and panics.
+func TestValidateNilRuleChallenge(t *testing.T) {
+	i := &Impl{Algorithm: "fast"}
+	lg := slog.With()
+
+	// This is the exact response for SHA256("hunter" + "0") with 0 leading zeros required.
+	const challengeStr = "hunter"
+	const response = "2652bdba8fb4d2ab39ef28d8534d7694c557a4ae146c1e9237bd8d950280500e"
+
+	req := mkRequest(t, map[string]string{
+		"nonce":       "0",
+		"elapsedTime": "69",
+		"response":    response,
+	})
+
+	for _, tc := range []struct {
+		name  string
+		input *challenge.ValidateInput
+	}{
+		{
+			name: "nil-rule-challenge",
+			input: &challenge.ValidateInput{
+				Rule:      &policy.Bot{},
+				Challenge: &challenge.Challenge{RandomData: challengeStr},
+			},
+		},
+		{
+			name: "nil-rule",
+			input: &challenge.ValidateInput{
+				Challenge: &challenge.Challenge{RandomData: challengeStr},
+			},
+		},
+		{
+			name:  "nil-challenge",
+			input: &challenge.ValidateInput{Rule: &policy.Bot{Challenge: &config.ChallengeRules{Algorithm: "fast"}}},
+		},
+		{
+			name:  "nil-input",
+			input: nil,
+		},
+	} {
+		t.Run(tc.name, func(t *testing.T) {
+			err := i.Validate(req, lg, tc.input)
+			if !errors.Is(err, challenge.ErrInvalidInput) {
+				t.Fatalf("expected ErrInvalidInput, got: %v", err)
+			}
+		})
+	}
+}
+
 func TestBasic(t *testing.T) {
 	i := &Impl{Algorithm: "fast"}
 	bot := &policy.Bot{

lib/http.go

@@ -222,8 +222,12 @@ func (s *Server) RenderIndex(w http.ResponseWriter, r *http.Request, cr policy.C
 	chall, err := s.issueChallenge(r.Context(), r, lg, cr, rule)
 	if err != nil {
 		lg.Error("can't get challenge", "err", err)
+		algorithm := "unknown"
+		if rule.Challenge != nil {
+			algorithm = rule.Challenge.Algorithm
+		}
 		s.ClearCookie(w, CookieOpts{Name: anubis.TestCookieName, Host: r.Host})
-		s.respondWithError(w, r, fmt.Sprintf("%s: %s", localizer.T("internal_server_error"), rule.Challenge.Algorithm), makeCode(err))
+		s.respondWithError(w, r, fmt.Sprintf("%s: %s", localizer.T("internal_server_error"), algorithm), makeCode(err))
 		return
 	}
 
@@ -248,9 +252,13 @@ func (s *Server) RenderIndex(w http.ResponseWriter, r *http.Request, cr policy.C
 
 	impl, ok := challenge.Get(chall.Method)
 	if !ok {
-		lg.Error("check failed", "err", "can't get algorithm", "algorithm", rule.Challenge.Algorithm)
+		algorithm := "unknown"
+		if rule.Challenge != nil {
+			algorithm = rule.Challenge.Algorithm
+		}
+		lg.Error("check failed", "err", "can't get algorithm", "algorithm", algorithm)
 		s.ClearCookie(w, CookieOpts{Name: anubis.TestCookieName, Host: r.Host})
-		s.respondWithError(w, r, fmt.Sprintf("%s: %s", localizer.T("internal_server_error"), rule.Challenge.Algorithm), makeCode(err))
+		s.respondWithError(w, r, fmt.Sprintf("%s: %s", localizer.T("internal_server_error"), algorithm), makeCode(err))
 		return
 	}
 

lib/policy/celchecker_test.go

@@ -1,44 +0,0 @@
-package policy
-
-import (
-	"net/http"
-	"testing"
-
-	"github.com/TecharoHQ/anubis/internal/dns"
-	"github.com/TecharoHQ/anubis/lib/config"
-	"github.com/TecharoHQ/anubis/lib/store/memory"
-)
-
-func newTestDNS(t *testing.T) *dns.Dns {
-	t.Helper()
-
-	ctx := t.Context()
-	memStore := memory.New(ctx)
-	cache := dns.NewDNSCache(300, 300, memStore)
-	return dns.New(ctx, cache)
-}
-
-func TestCELChecker_MapIterationWrappers(t *testing.T) {
-	cfg := &config.ExpressionOrList{
-		Expression: `headers.exists(k, k == "Accept") && query.exists(k, k == "format")`,
-	}
-
-	checker, err := NewCELChecker(cfg, newTestDNS(t))
-	if err != nil {
-		t.Fatalf("creating CEL checker failed: %v", err)
-	}
-
-	req, err := http.NewRequest(http.MethodGet, "https://example.com/?format=json", nil)
-	if err != nil {
-		t.Fatalf("making request failed: %v", err)
-	}
-	req.Header.Set("Accept", "application/json")
-
-	got, err := checker.Check(req)
-	if err != nil {
-		t.Fatalf("checking expression failed: %v", err)
-	}
-	if !got {
-		t.Fatal("expected expression to evaluate true")
-	}
-}

lib/policy/expressions/http_headers.go

@@ -66,9 +66,7 @@ func (h HTTPHeaders) Get(key ref.Val) ref.Val {
 	return result
 }
 
-func (h HTTPHeaders) Iterator() traits.Iterator {
-	return newMapIterator(h.Header)
-}
+func (h HTTPHeaders) Iterator() traits.Iterator { panic("TODO(Xe): implement me") }
 
 func (h HTTPHeaders) IsZeroValue() bool {
 	return len(h.Header) == 0

lib/policy/expressions/map_iterator.go

@@ -1,60 +0,0 @@
-package expressions
-
-import (
-	"errors"
-	"maps"
-	"reflect"
-	"slices"
-
-	"github.com/google/cel-go/common/types"
-	"github.com/google/cel-go/common/types/ref"
-	"github.com/google/cel-go/common/types/traits"
-)
-
-var ErrNotImplemented = errors.New("expressions: not implemented")
-
-type stringSliceIterator struct {
-	keys []string
-	idx  int
-}
-
-func (s *stringSliceIterator) Value() any {
-	return s
-}
-
-func (s *stringSliceIterator) ConvertToNative(typeDesc reflect.Type) (any, error) {
-	return nil, ErrNotImplemented
-}
-
-func (s *stringSliceIterator) ConvertToType(typeValue ref.Type) ref.Val {
-	return types.NewErr("can't convert from %q to %q", types.IteratorType, typeValue)
-}
-
-func (s *stringSliceIterator) Equal(other ref.Val) ref.Val {
-	return types.NewErr("can't compare %q to %q", types.IteratorType, other.Type())
-}
-
-func (s *stringSliceIterator) Type() ref.Type {
-	return types.IteratorType
-}
-
-func (s *stringSliceIterator) HasNext() ref.Val {
-	return types.Bool(s.idx < len(s.keys))
-}
-
-func (s *stringSliceIterator) Next() ref.Val {
-	if s.HasNext() != types.True {
-		return nil
-	}
-
-	val := s.keys[s.idx]
-	s.idx++
-	return types.String(val)
-}
-
-func newMapIterator(m map[string][]string) traits.Iterator {
-	return &stringSliceIterator{
-		keys: slices.Collect(maps.Keys(m)),
-		idx:  0,
-	}
-}

lib/policy/expressions/url_values.go

@@ -1,6 +1,7 @@
 package expressions
 
 import (
+	"errors"
 	"net/url"
 	"reflect"
 	"strings"
@@ -10,6 +11,8 @@ import (
 	"github.com/google/cel-go/common/types/traits"
 )
 
+var ErrNotImplemented = errors.New("expressions: not implemented")
+
 // URLValues is a type wrapper to expose url.Values into CEL programs.
 type URLValues struct {
 	url.Values
@@ -66,9 +69,7 @@ func (u URLValues) Get(key ref.Val) ref.Val {
 	return result
 }
 
-func (u URLValues) Iterator() traits.Iterator {
-	return newMapIterator(u.Values)
-}
+func (u URLValues) Iterator() traits.Iterator { panic("TODO(Xe): implement me") }
 
 func (u URLValues) IsZeroValue() bool {
 	return len(u.Values) == 0