From 97d15cd80340d4cfbbe07fab92f0228852619e61 Mon Sep 17 00:00:00 2001 From: Xe Iaso Date: Mon, 18 May 2026 21:26:40 -0400 Subject: [PATCH] fix(expressions): validate randInt bounds before rand.IntN Non-positive or platform-overflowing arguments to the CEL randInt helper used to reach rand.IntN unchecked, surfacing a CEL evaluator error during request processing when policies passed attacker-influenced values (e.g. contentLength). Reject non-positive bounds and detect int narrowing explicitly, returning a typed CEL error in both cases. Ref: AWOO-010 Signed-off-by: Xe Iaso --- docs/docs/CHANGELOG.md | 1 + lib/policy/expressions/environment.go | 11 +++- lib/policy/expressions/environment_test.go | 71 ++++++++++++++++++++++ 3 files changed, 82 insertions(+), 1 deletion(-) diff --git a/docs/docs/CHANGELOG.md b/docs/docs/CHANGELOG.md index 1b53f06b..98b40b3f 100644 --- a/docs/docs/CHANGELOG.md +++ b/docs/docs/CHANGELOG.md @@ -35,6 +35,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Fix an edge case where load average expression values could nil pointer dereference when Anubis just started up. - Fix an obscure case where Anubis in subrequest mode could allow redirects to invalid domains with strange instructions. - Fix `path_regex` and CEL `path` rules not matching when using Traefik `forwardAuth` middleware. Anubis now checks `X-Forwarded-Uri` (Traefik) in addition to `X-Original-URI` (nginx) when resolving the request path in subrequest mode ([#1628](https://github.com/TecharoHQ/anubis/issues/1628)). +- Validate bounds in the CEL `randInt` helper so non-positive or platform-overflowing arguments surface a typed CEL error instead of an evaluator panic. ## v1.25.0: Necron diff --git a/lib/policy/expressions/environment.go b/lib/policy/expressions/environment.go index 1e583a92..87027ea5 100644 --- a/lib/policy/expressions/environment.go +++ b/lib/policy/expressions/environment.go @@ -222,7 +222,16 @@ func New(opts ...cel.EnvOption) (*cel.Env, error) { return types.ValOrErr(val, "value is not an integer, but is %T", val) } - return types.Int(rand.IntN(int(n))) + if n <= 0 { + return types.NewErr("randInt bound must be positive, got %d", int64(n)) + } + + bound := int(n) + if types.Int(bound) != n { + return types.NewErr("randInt bound %d overflows platform int", int64(n)) + } + + return types.Int(rand.IntN(bound)) }), ), ), diff --git a/lib/policy/expressions/environment_test.go b/lib/policy/expressions/environment_test.go index c33fbbcb..c814fd0e 100644 --- a/lib/policy/expressions/environment_test.go +++ b/lib/policy/expressions/environment_test.go @@ -9,6 +9,7 @@ import ( "github.com/TecharoHQ/anubis/internal/dns" "github.com/TecharoHQ/anubis/lib/store/memory" + "github.com/google/cel-go/cel" "github.com/google/cel-go/common/types" "github.com/google/cel-go/common/types/ref" ) @@ -688,6 +689,14 @@ func TestNewEnvironment(t *testing.T) { description: "should return values in correct range", shouldCompile: true, }, + { + name: "randInt-large-bound", + expression: `randInt(2147483647) >= 0`, + variables: map[string]any{}, + expectBool: boolPtr(true), + description: "should accept int32-max bounds without overflow", + shouldCompile: true, + }, { name: "strings-extension-size", expression: `"hello".size() == 5`, @@ -750,3 +759,65 @@ func TestNewEnvironment(t *testing.T) { func boolPtr(b bool) *bool { return &b } + +func TestRandIntInvalidBounds(t *testing.T) { + env, err := New(cel.Variable("contentLength", cel.IntType)) + if err != nil { + t.Fatalf("failed to create environment: %v", err) + } + + tests := []struct { + name string + expression string + variables map[string]any + wantErrText string + description string + }{ + { + name: "zero-bound-literal", + expression: `randInt(0)`, + variables: map[string]any{}, + wantErrText: "randInt bound must be positive", + description: "randInt(0) should return a CEL error, not panic", + }, + { + name: "negative-bound-literal", + expression: `randInt(-5)`, + variables: map[string]any{}, + wantErrText: "randInt bound must be positive", + description: "randInt(-5) should return a CEL error, not panic", + }, + { + name: "zero-bound-from-variable", + expression: `randInt(contentLength)`, + variables: map[string]any{"contentLength": 0}, + wantErrText: "randInt bound must be positive", + description: "attacker-controlled zero contentLength should error gracefully", + }, + { + name: "negative-bound-from-variable", + expression: `randInt(contentLength)`, + variables: map[string]any{"contentLength": -1}, + wantErrText: "randInt bound must be positive", + description: "attacker-controlled negative contentLength should error gracefully", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + prog, err := Compile(env, tt.expression) + if err != nil { + t.Fatalf("failed to compile expression %q: %v", tt.expression, err) + } + + result, _, err := prog.Eval(tt.variables) + if err == nil { + t.Fatalf("%s: expected an evaluation error, got result %v", tt.description, result) + } + + if !strings.Contains(err.Error(), tt.wantErrText) { + t.Errorf("%s: expected error containing %q, got %q", tt.description, tt.wantErrText, err.Error()) + } + }) + } +}